geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jinmei Liao <jil...@pivotal.io>
Subject Re: Review Request 58393: GEODE-2686: Remove JarClassLoader
Date Thu, 13 Apr 2017 15:25:17 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58393/#review171878
-----------------------------------------------------------




geode-core/build.gradle
Lines 116 (patched)
<https://reviews.apache.org/r/58393/#comment244876>

    put the version number in the dependency-versions.properties



geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java
Lines 155 (patched)
<https://reviews.apache.org/r/58393/#comment244877>

    the other 2 methods are hasValidJarContent, maybe make this to be the same name as well?



geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java
Lines 368 (patched)
<https://reviews.apache.org/r/58393/#comment244878>

    since we logs the entire stack trace of the exception, we know the exact caues of the
problem anyway, can we collapse those catch blocks?



geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
Line 48 (original), 54 (patched)
<https://reviews.apache.org/r/58393/#comment244879>

    If they only refer to empty string now, can we just get rid of them?



geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderIntegrationTest.java
Lines 81 (patched)
<https://reviews.apache.org/r/58393/#comment244880>

    you can use:
    
    extLibsDir = temporaryFolder.newFolder("ext");
    
    It makes sure the dir is created too.



geode-core/src/test/java/org/apache/geode/management/DeployJarTestSuite.java
Lines 27 (patched)
<https://reviews.apache.org/r/58393/#comment244881>

    by checking in this suite, would it cause the tests to be run twice?



geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
Lines 250 (patched)
<https://reviews.apache.org/r/58393/#comment244882>

    after you do an execute, gfshConnector.getGfshOutput() will always give you the last result
string printed out in gfsh. 
    you don't need commandResultToString method here, I believe.


- Jinmei Liao


On April 12, 2017, 6:06 p.m., Jared Stewart wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58393/
> -----------------------------------------------------------
> 
> (Updated April 12, 2017, 6:06 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Ken Howe, Kirk Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-2686: Remove JarClassLoader
> - Remove JarClassLoader
> - Replace ClassPathLoader's collection of JarClassLoaders with a single URLClassLoader
> - Change naming scheme for deployed jars from 'vf.gf#myJar.jar#1' to 'myJar.v1.jar'
> 
> GEODE-2705: Jars undeployed from cluster configuration will not be loaded from disk on
member restart
> 
> GEODE-2686: Add more logging to JarDeployer
> 
> GEODE-2290: Limit scanning of deployed jars
> - Uses fast-classpath-scanner to scan jars for classes containing Functions without eagerly
loading all classes in the jar.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/build.gradle 304a1c4d7ede5b82299c8a0f05038f3d11b2ac82 
>   geode-assembly/src/test/resources/expected_jars.txt b7d1dc22fd7995e6f7786984994ce62e1064bed3

>   geode-core/build.gradle 757599a9d76844dec17e545d1b4d19b32c22afef 
>   geode-core/src/main/java/org/apache/geode/internal/ClassPathLoader.java 9ab7c308579c9ea5c8fb300ee7550b3153507b1c

>   geode-core/src/main/java/org/apache/geode/internal/DeployedJar.java PRE-CREATION 
>   geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java f4f40692d1574bb9eca9bf8f7d42ce64fab18f9b

>   geode-core/src/main/java/org/apache/geode/internal/JarClassLoader.java 9cd05899898fc82eec87a3f0ab5ace31b67ef308

>   geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java 18d4b42213f29761792d35f12eddfb1cbfbba081

>   geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
f904af188eb295f1cb84e41023de168c160ed07f 
>   geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 08f916b117a07ed303638366c64a2c9d5b807b91

>   geode-core/src/main/java/org/apache/geode/internal/cache/persistence/BackupManager.java
d052551d85e5b01b16255160f76795e829a14429 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DeployFunction.java
5f1f16167e08dd79b2995555c42541fb01ee2546 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ListDeployedFunction.java
8df24dbe35a08173932776226c9f20665db5e9a3 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/UndeployFunction.java
3f0508270b51b4930c2eafe15547b9b258b96bd9 
>   geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderIntegrationTest.java
c52d5754792cfeaed1bb87bdb9380a360308395a 
>   geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderTest.java 6baddaf4b9d3b58fce10fc224440111c2e88b849

>   geode-core/src/test/java/org/apache/geode/internal/DeployedJarJUnitTest.java PRE-CREATION

>   geode-core/src/test/java/org/apache/geode/internal/JarClassLoaderJUnitTest.java adc1d2eb2b633d4c81076db2b4685fedb6315913

>   geode-core/src/test/java/org/apache/geode/internal/JarDeployerDUnitTest.java a365899c46bb7f28ce75118e45137a243493cbd0

>   geode-core/src/test/java/org/apache/geode/internal/JarDeployerIntegrationTest.java
71daeccc42e7944f01ce50d94fc3c4afc5c9b121 
>   geode-core/src/test/java/org/apache/geode/internal/cache/IncrementalBackupDUnitTest.java
0cc003ebd15628ea62a67a36c1ac595fe0a02693 
>   geode-core/src/test/java/org/apache/geode/management/DeployJarTestSuite.java PRE-CREATION

>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
7b0823bea38043967502e0accee90963d5d7b650 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserCommandsDUnitTest.java
24f5cdb549fbc172e12f965044681e83f11b5829 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java
c3d7f5ec2483b4ae28f73e3b3477180d20b1a53e 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java
cecc8cfd683a2fb24d621c9010d5650ef2996966 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
7cc84d6c4417872daf9a13ea5f237e458cafe154 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
02bad30e97f0a3550eac0bf00ce0c6833d761dae 
>   geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java
5f4cd74b8af81ae3bbeff04d6ce44a5c4ac9ec59 
> 
> 
> Diff: https://reviews.apache.org/r/58393/diff/1/
> 
> 
> Testing
> -------
> 
> - Precheckin passed
> - Manually deployed jars and executed functions via gfsh
> 
> 
> Thanks,
> 
> Jared Stewart
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message