You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Sai Boorlagadda <sb...@pivotal.io> on 2016/06/29 23:53:44 UTC
Review Request 49409: GEODE-1573: Use snappy java implementation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/
-----------------------------------------------------------
Review request for geode, Darrel Schneider and Dan Smith.
Bugs: GEODE-1573
https://issues.apache.org/jira/browse/GEODE-1573
Repository: geode
Description
-------
* fixed bundled jars test failure
* removed swalloing exceptio on unsupported OS
* removed unwanted gradle dependency
Diffs
-----
geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
Diff: https://reviews.apache.org/r/49409/diff/
Testing
-------
prechecin
Thanks,
Sai Boorlagadda
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/#review140448
-----------------------------------------------------------
Fix it, then Ship it!
fix and ship
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java (line 42)
<https://reviews.apache.org/r/49409/#comment205833>
Change "Starting" to "as of" and put a comma after "1.0". Also I think "Geode" is better than "GEODE".
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java (line 89)
<https://reviews.apache.org/r/49409/#comment205836>
I know we talked about this but since you are changing all these tests anyway (getting rid of the "try/catch") it seems like you should go ahead and change them to use the constructor instead of calling getDefaultInstance
- Darrel Schneider
On July 1, 2016, 11:53 a.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49409/
> -----------------------------------------------------------
>
> (Updated July 1, 2016, 11:53 a.m.)
>
>
> Review request for geode, Darrel Schneider and Dan Smith.
>
>
> Bugs: GEODE-1573
> https://issues.apache.org/jira/browse/GEODE-1573
>
>
> Repository: geode
>
>
> Description
> -------
>
> * fixed bundled jars test failure
> * removed swalloing exceptio on unsupported OS
> * removed unwanted gradle dependency
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
> geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
> geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
> geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
> gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
>
> Diff: https://reviews.apache.org/r/49409/diff/
>
>
> Testing
> -------
>
> prechecin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/#review140464
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On July 1, 2016, 4:49 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49409/
> -----------------------------------------------------------
>
> (Updated July 1, 2016, 4:49 p.m.)
>
>
> Review request for geode, Darrel Schneider and Dan Smith.
>
>
> Bugs: GEODE-1573
> https://issues.apache.org/jira/browse/GEODE-1573
>
>
> Repository: geode
>
>
> Description
> -------
>
> * fixed bundled jars test failure
> * removed swalloing exceptio on unsupported OS
> * removed unwanted gradle dependency
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
> geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
> geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
> geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerOffHeapDUnitTest.java 56ce3039f970cf8b9cbd14ed0a36305ff87ddcd7
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsOffHeapDUnitTest.java 0bfa91096bde07614802ffd2d10f2899200f239e
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
> gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
>
> Diff: https://reviews.apache.org/r/49409/diff/
>
>
> Testing
> -------
>
> prechecin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/
-----------------------------------------------------------
(Updated July 1, 2016, 11:49 p.m.)
Review request for geode, Darrel Schneider and Dan Smith.
Bugs: GEODE-1573
https://issues.apache.org/jira/browse/GEODE-1573
Repository: geode
Description
-------
* fixed bundled jars test failure
* removed swalloing exceptio on unsupported OS
* removed unwanted gradle dependency
Diffs (updated)
-----
geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerOffHeapDUnitTest.java 56ce3039f970cf8b9cbd14ed0a36305ff87ddcd7
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsOffHeapDUnitTest.java 0bfa91096bde07614802ffd2d10f2899200f239e
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
Diff: https://reviews.apache.org/r/49409/diff/
Testing
-------
prechecin
Thanks,
Sai Boorlagadda
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/
-----------------------------------------------------------
(Updated July 1, 2016, 6:53 p.m.)
Review request for geode, Darrel Schneider and Dan Smith.
Changes
-------
fixed review comments
Bugs: GEODE-1573
https://issues.apache.org/jira/browse/GEODE-1573
Repository: geode
Description
-------
* fixed bundled jars test failure
* removed swalloing exceptio on unsupported OS
* removed unwanted gradle dependency
Diffs (updated)
-----
geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
Diff: https://reviews.apache.org/r/49409/diff/
Testing
-------
prechecin
Thanks,
Sai Boorlagadda
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Sai Boorlagadda <sb...@pivotal.io>.
> On June 30, 2016, 5:21 p.m., Darrel Schneider wrote:
> > geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java, line 89
> > <https://reviews.apache.org/r/49409/diff/1/?file=1433744#file1433744line89>
> >
> > I would change all the unit tests that call getDefaultInstance to instead do new SnappyCompressor();
We could fix tests to call constructor when we removed deprecated methods.
- Sai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/#review140187
-----------------------------------------------------------
On June 29, 2016, 11:53 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49409/
> -----------------------------------------------------------
>
> (Updated June 29, 2016, 11:53 p.m.)
>
>
> Review request for geode, Darrel Schneider and Dan Smith.
>
>
> Bugs: GEODE-1573
> https://issues.apache.org/jira/browse/GEODE-1573
>
>
> Repository: geode
>
>
> Description
> -------
>
> * fixed bundled jars test failure
> * removed swalloing exceptio on unsupported OS
> * removed unwanted gradle dependency
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
> geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
> geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
> geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
> gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
>
> Diff: https://reviews.apache.org/r/49409/diff/
>
>
> Testing
> -------
>
> prechecin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 49409: GEODE-1573: Use snappy java implementation
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49409/#review140187
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java (line 31)
<https://reviews.apache.org/r/49409/#comment205505>
Is "Utilizes the xerial" still correct?
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java (line 44)
<https://reviews.apache.org/r/49409/#comment205504>
remove the @throws in the javadoc comment
geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java (line 50)
<https://reviews.apache.org/r/49409/#comment205508>
What do you think of deprecating this method?
Why should we have only one instance of this class? It is odd that the old constructor synchronized on a static. Seems like it should have just had a static block that did the initialization code when the class was loaded.
But in the new world we should encourage anyone getting an instance of this to just call the constructor.
geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java (line 89)
<https://reviews.apache.org/r/49409/#comment205509>
I would change all the unit tests that call getDefaultInstance to instead do new SnappyCompressor();
- Darrel Schneider
On June 29, 2016, 4:53 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49409/
> -----------------------------------------------------------
>
> (Updated June 29, 2016, 4:53 p.m.)
>
>
> Review request for geode, Darrel Schneider and Dan Smith.
>
>
> Bugs: GEODE-1573
> https://issues.apache.org/jira/browse/GEODE-1573
>
>
> Repository: geode
>
>
> Description
> -------
>
> * fixed bundled jars test failure
> * removed swalloing exceptio on unsupported OS
> * removed unwanted gradle dependency
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle ce303bb5514b25566cc6c4b44831b0dcaccd59fd
> geode-assembly/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/LauncherLifecycleCommandsIntegrationTest.java 22c6947f1a76131977282b91905fa73bd021d87a
> geode-assembly/src/test/resources/expected_jars.txt 6104dbba8a8b49510ce4992644ca6606a5bc7e2a
> geode-core/build.gradle af5c02bed9eacf9c90bf3fb5238124e56ed40e6d
> geode-core/src/main/java/com/gemstone/gemfire/compression/SnappyCompressor.java 512756a41b78e8b4eb4867d91f312d8328a62ab2
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheConfigDUnitTest.java a757c5675d16a145a500b41fae9e5c2389695410
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionCacheListenerDUnitTest.java f649d9aa87cd87e52f7ffae90ee629dd5e10fffd
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionConfigDUnitTest.java 93c8c0044274a741886f22db7d5a5c53f1ccefee
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionFactoryDUnitTest.java 43f7d2313d5d9ea2227a76481d9e011d95a0674a
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/CompressionRegionOperationsDUnitTest.java 1f568599ed14ab09aeefdf0b5fbeb2c0fc7e75c8
> geode-core/src/test/java/com/gemstone/gemfire/internal/compression/SnappyCompressorJUnitTest.java ae8bf7be81e4bb178783c0d1ea041d47f31a8824
> gradle/dependency-versions.properties 9de665c850d472d918a129c17447f857400580f0
>
> Diff: https://reviews.apache.org/r/49409/diff/
>
>
> Testing
> -------
>
> prechecin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>