Management tests failing on develop

[Kirk Lund]

I merged in 16 PRs today and precheckin was clean for each individually. I
launched a precheckin after all were merged in and that seems to have tons
of Management test failures. So on Monday, I'll try to identify which PRs
aren't playing nicely with each other and start backing them out. Sorry the
tests are going to be so messy for the weekend.

I think this is one of the times where trying to separate a huge
refactoring out into many tickets just didn't work. Sorry code reviewers,
but some of these refactorings are going to have to be HUGE change-sets.

:geode-core:integrationTest

org.apache.geode.management.internal.cli.help.HelperIntegrationTest >
testHelpWithNoInput FAILED
java.lang.AssertionError:
Expected size:<2> but was:<4> in:
<["help (Available)",
"Display syntax and usage information for all commands or list all
available commands if  isn't specified.",
"hint (Available)",
"Provide hints for a topic or list all available topics if "topic"
isn't specified."]>
at
org.apache.geode.management.internal.cli.help.HelperIntegrationTest.testHelpWithNoInput(HelperIntegrationTest.java:69)

org.apache.geode.management.internal.cli.help.HelperIntegrationTest >
testHintWithNoInput FAILED
java.lang.AssertionError:
Expected size:<21> but was:<22> in:
<["Hints are available for the following topics. Use "hint
" for a specific hint.",
"",
"Client",
"Cluster Configuration",
"Configuration",
"Data",
"Debug-Utility",
"Disk Store",
"Function Execution",
"GFSH",
"Geode",
"Help",
"JMX",
"Lifecycle",
"Locator",
"Logs",
"Management-Monitoring",
"Manager",
"Region",
"Server",
"Statistics",
"WAN"]>
at
org.apache.geode.management.internal.cli.help.HelperIntegrationTest.testHintWithNoInput(HelperIntegrationTest.java:96)

org.apache.geode.management.internal.cli.GfshParserConverterTest >
testHintConverter FAILED
java.lang.StringIndexOutOfBoundsException: String index out of range: -1
at java.lang.String.substring(String.java:1931)
at
org.apache.geode.management.internal.cli.GfshParser.lambda$completeAdvanced$0(GfshParser.java:265)
at java.util.ArrayList.replaceAll(ArrayList.java:1442)
at
org.apache.geode.management.internal.cli.GfshParser.completeAdvanced(GfshParser.java:264)
at
org.apache.geode.test.dunit.rules.GfshParserRule.complete(GfshParserRule.java:58)
at
org.apache.geode.management.internal.cli.GfshParserConverterTest.testHintConverter(GfshParserConverterTest.java:126)

org.apache.geode.management.internal.security.CliCommandsSecurityTest >
testNoAccess FAILED
org.assertj.core.api.SoftAssertionError:
The following assertion failed:
1) [destroy function --id=InterestCalculations]
Expecting:
 <"stranger not authorized for CLUSTER:MANAGE:JAR">
to contain:
 <"DATA:MANAGE">
at CliCommandsSecurityTest.testNoAccess(CliCommandsSecurityTest.java:74)
at
org.assertj.core.api.SoftAssertions.assertAll(SoftAssertions.java:134)
at
org.apache.geode.management.internal.security.CliCommandsSecurityTest.testNoAccess(CliCommandsSecurityTest.java:78)

org.apache.geode.management.internal.security.GfshCommandsSecurityTest >
testRegionAReader FAILED
org.junit.ComparisonFailure: [destroy function
--id=InterestCalculations] expected:<[110]> but was:<[415]>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
Method)
at
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at
org.apache.geode.management.internal.security.GfshCommandsSecurityTest.runCommandsPermittedAndForbiddenBy(GfshCommandsSecurityTest.java:164)
at
org.apache.geode.management.internal.security.GfshCommandsSecurityTest.testRegionAReader(GfshCommandsSecurityTest.java:108)

org.apache.geode.management.internal.security.GfshCommandsSecurityTest >
testRegionAWriter FAILED
org.junit.ComparisonFailure: [destroy function
--id=InterestCalculations] expected:<[110]> but was:<[415]>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native
Method)
at
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at
org.apache.geode.management.internal.security.GfshCommandsSecurityTest.runCommandsPermittedAndForbiddenBy(GfshCommandsSecurityTest.java:164)
at
org.apache.geode.management.internal.security.GfshCommandsSecurityTest.testRegionAWriter(GfshCommandsSecurityTest.java:114)

org.apache.geode.management.internal.security.GfshCommandsSecurityTest >
testClusterReader FAILED

Re: BCSnapshotDUnitTest failing

[Kirk Lund]

Yep you're right. Thanks Mark! Too bad there's no delete in email...

On Fri, Aug 11, 2017 at 4:31 PM, Mark Bretl  wrote:

> Hi Kirk,
>
> There is no 'pivotalgf-assembly' project in Geode. Looks like it is part of
> a different distribution. ;)
>
> --Mark
>
> On Fri, Aug 11, 2017 at 4:09 PM, Kirk Lund  wrote:
>
> > BCSnapshotDUnitTest seems to be failing intermittently(?) on develop.
> >
> > :pivotalgf-assembly:distributedTest
> >
> > org.apache.geode.BCSnapshotDUnitTest > testSnapshot[0] FAILED
> > org.apache.geode.test.dunit.RMIException: While invoking
> > org.apache.geode.BCSnapshotDUnitTest$1.run in VM 0 running on Host
> > 3dfb24d73711 with 4 VMs
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
> > at org.apache.geode.test.dunit.VM.invoke(VM.java:302)
> > at
> > org.apache.geode.BCSnapshotDUnitTest.validateOldSnapshot(
> > BCSnapshotDUnitTest.java:103)
> > at
> > org.apache.geode.BCSnapshotDUnitTest.testSnapshot(
> > BCSnapshotDUnitTest.java:95)
> >
> > Caused by:
> > java.io.FileNotFoundException: No snapshot files found in
> > /tmp/build/ae3c03f4/gemfire/closed/pivotalgf-assembly/
> > build/distributedTest437/diskDir/disk-1/bcCacheSnapshotDir
> >
> > 14 tests completed, 1 failed, 1 skipped
> > :pivotalgf-assembly:distributedTest FAILED
> >
>

Build failed in Jenkins: Geode-nightly-flaky #88

[Apache Jenkins Server]

See 


Changes:

[dbarnes] GEODE-3328 Properties to set Key/Trust Store Type for SSL 
configuration

[huynhja] GEODE-3397: Fixed issue with Tomcat locators in cache-client.xml file

[jiliao] GEODE-3328: refactor ConnectCommand

[dbarnes] GEODE-3396 pub-tools support for product name & version variables,

[klund] GEODE-3413: overhaul launcher and process classes and tests

[kmiller] GEODE-3410 Doc update for gfsh query command changes

[gzhou] GEODE-3304: when colocated child bucket is destroyed, it should not

[dschneider] GEODE-3300: Complete and expose parallel export feature for use

[dbarnes] GEODE-3395 Variable-ize product version and name in user guide -

[klund] GEODE-3230: Cleaning up unused (Cli)Strings

[klund] GEODE-3255: Refactor CreateAlterDestroyRegionCommands and tests

[klund] GEODE-3257: Refactoring DeployCommands

[klund] GEODE-3266: Refactoring PDXCommands

[klund] GEODE-3268: Refactoring RegionCommands

[klund] GEODE-3267: Refactoring QueueCommands - updated based on feedback

[klund] GEODE-3270: Refactoring (renaming) StatusCommands

[klund] GEODE-3267: Refactoring QueueCommands

[klund] GEODE-3261: Refactoring GfshHelpCommands

[klund] GEODE-3265: Refactoring MiscellaneousCommands

[klund] GEODE-3337: Refactoring LauncherLifecycleCommandsDUnitTest

[klund] GEODE-3258: Refactoring DiskStoreCommands

[klund] GEODE-3260: Refactoring FunctionCommands

[klund] GEODE-3262: Refactoring IndexCommands

[bschuchardt] GEODE-3403: Modify rolling upgrade test configurations for 1.2.x 
release

[klund] GEODE-3254: Refactoring ConfigCommands

[klund] GEODE-3259: Refactoring DurableClientCommands

[klund] GEODE-3264: Refactoring MemberCommands

[bschuchardt] GEODE-3427: CI failure in

[dbarnes] GEODE-3395 Variable-ize product version and name in user guide - 
Getting

--
[...truncated 110.67 KB...]
Download 
https://repo1.maven.org/maven2/org/springframework/hateoas/spring-hateoas/0.23.0.RELEASE/spring-hateoas-0.23.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/scala-lang/scala-library/2.10.6/scala-library-2.10.6.jar
Download 
https://repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.10.6/scala-reflect-2.10.6.jar
Download 
https://repo1.maven.org/maven2/com/fasterxml/jackson/module/jackson-module-paranamer/2.8.6/jackson-module-paranamer-2.8.6.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-annotations/1.5.10/swagger-annotations-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/swagger/swagger-models/1.5.10/swagger-models-1.5.10.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spi/2.6.1/springfox-spi-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-schema/2.6.1/springfox-schema-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-swagger-common/2.6.1/springfox-swagger-common-2.6.1.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-spring-web/2.6.1/springfox-spring-web-2.6.1.jar
Download 
https://repo1.maven.org/maven2/com/fasterxml/classmate/1.3.1/classmate-1.3.1.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-core/1.2.0.RELEASE/spring-plugin-core-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/springframework/plugin/spring-plugin-metadata/1.2.0.RELEASE/spring-plugin-metadata-1.2.0.RELEASE.jar
Download 
https://repo1.maven.org/maven2/org/mapstruct/mapstruct/1.0.0.Final/mapstruct-1.0.0.Final.jar
Download 
https://repo1.maven.org/maven2/com/thoughtworks/paranamer/paranamer/2.8/paranamer-2.8.jar
Download 
https://repo1.maven.org/maven2/io/springfox/springfox-core/2.6.1/springfox-core-2.6.1.jar
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:geode-web-api:processResources
:geode-web-api:classes
:geode-assembly:docs
:geode-assembly:gfshDepsJar
:geode-common:javadocJar
:geode-common:sourcesJar
:geode-common:signArchives SKIPPED
:geode-core:javadocJar
:geode-core:raJar
:geode-core:jcaJar
:geode-core:sourcesJar
:geode-core:signArchives SKIPPED
:geode-core:webJar
:geode-cq:jar
:geode-cq:javadoc
:geode-cq:javadocJar
:geode-cq:sourcesJar
:geode-cq:signArchives SKIPPED
:geode-json:javadocJar
:geode-json:sourcesJar
:geode-json:signArchives SKIPPED
:geode-lucene:jar
:geode-lucene:javadoc
:geode-lucene:javadocJar
:geode-lucene:sourcesJar
:geode-lucene:signArchives SKIPPED
:geode-old-client-support:jar
:geode-old-client-support:javadoc
:geode-old-client-support:javadocJar
:geode-old-client-support:sourcesJar
:geode-old-client-support:signArchives SKIPPED
:geode-protobuf:jar
:geode-protobuf:javadoc
:geode-protobuf:javadocJar
:geode-protobuf:sourcesJar
:geode-protobuf:signArchives SKIPPED
:geode-pulse:javadoc
:geode-pulse:javadocJar
:geode-pulse:sourcesJar
:geode-pulse:war
:geode-pulse:signArchives SKIPPED
:geode-rebalancer:jar
:geode-rebalancer:javadoc

[GitHub] geode pull request #710: GEODE-3435: Fix serialization test failure

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/710


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Build failed in Jenkins: Geode-nightly #920

[Apache Jenkins Server]

See 


Changes:

[jstewart] GEODE-3313: Test utility supports building jar files with multiple

[dbarnes] GEODE-3111 GatewayReceiver - DEFAULT_MANUAL_START value is ambiguous

[dbarnes] GEODE-3328 Properties to set Key/Trust Store Type for SSL 
configuration

[huynhja] GEODE-3397: Fixed issue with Tomcat locators in cache-client.xml file

[jiliao] GEODE-3328: refactor ConnectCommand

[dbarnes] GEODE-3396 pub-tools support for product name & version variables,

--
[...truncated 1.45 MB...]
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.GfshShellConnectionRule.connectAndVerify(GfshShellConnectionRule.java:113)
at 
org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest.connect(IndexCommandOverHttpTest.java:28)

org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest > 
testCannotCreateIndexWithExistingIndexName FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.GfshShellConnectionRule.connectAndVerify(GfshShellConnectionRule.java:113)
at 
org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest.connect(IndexCommandOverHttpTest.java:28)

org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest > 
testCannotCreateIndexWithInvalidIndexExpression FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.GfshShellConnectionRule.connectAndVerify(GfshShellConnectionRule.java:113)
at 
org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest.connect(IndexCommandOverHttpTest.java:28)

org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest > 
testCannotDestroyIndexWithInvalidIndexName FAILED
org.junit.ComparisonFailure: expected:<[tru]e> but was:<[fals]e>
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at 
sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at 
sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at 
org.apache.geode.test.dunit.rules.GfshShellConnectionRule.connectAndVerify(GfshShellConnectionRule.java:113)
at 
org.apache.geode.management.internal.cli.commands.IndexCommandOverHttpTest.connect(IndexCommandOverHttpTest.java:28)

org.apache.geode.management.internal.cli.commands.ExportLogsStatsOverHttpDUnitTest
 > classMethod FAILED
java.lang.AssertionError: Suspicious strings were written to the log during 
this run.
Fix the strings or use IgnoredException.addIgnoredException to ignore.
---
Found suspect string in log4j at line 538

[fatal 2017/08/12 00:33:34.645 UTC  tid=0x2847] 
SSL Error in connecting to peer /127.0.0.1[52,060].
javax.net.ssl.SSLException: Unrecognized SSL message, plaintext connection?
at 
sun.security.ssl.InputRecord.handleUnknownRecord(InputRecord.java:710)
at sun.security.ssl.InputRecord.read(InputRecord.java:527)
at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:983)
at 
sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1385)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1413)
at 
sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1397)
at 
org.apache.geode.internal.net.SocketCreator.configureServerSSLSocket(SocketCreator.java:1007)
at 
org.apache.geode.distributed.internal.tcpserver.TcpServer.lambda$processRequest$0(TcpServer.java:345)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)

---
Found suspect string in log4j at line 574

[error 2017/08/12 

[GitHub] geode pull request #710: GEODE-3435: Fix serialization test failure

[nreich]

GitHub user nreich opened a pull request:

https://github.com/apache/geode/pull/710

GEODE-3435: Fix serialization test failure

This pull request it to fix an issue introduced in GEODE-3300 that caused 
AnalyzeSerializablesJUnitTest to fail.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nreich/geode bug/GEODE-3435

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/710.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #710


commit d70be239bb94c4b8d610367b98deeafc9b224345
Author: Nick Reich 
Date:   2017-08-12T00:18:40Z

GEODE-3435: Fix serialization test failure




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: ParallelSnapshotFileMapper broke AnalyzeSerializablesJUnitTest

[Nick Reich]

I have created GEODE-3435 to fix the issue.

On Fri, Aug 11, 2017 at 3:41 PM, Kirk Lund  wrote:

> Looks like it was this commit...
>
> commit 7072f8ef7b764d1507e26cca8ed0c4d184ccc81a
> Author: Nick Reich 
> Date:   Fri Jul 28 16:47:10 2017 -0700
>
> GEODE-3300: Complete and expose parallel export feature for use
>
> This closes #704
>
>
> On Fri, Aug 11, 2017 at 3:40 PM, Kirk Lund  wrote:
>
> > Looks like one of the recent commits to develop resulted in this failure
> > involving org/apache/geode/internal/cache/snapshot/
> > ParallelSnapshotFileMapper...
> >
> > :geode-core:integrationTest
> >
> > org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest >
> > testSerializables FAILED
> > java.lang.AssertionError: New or moved classes---
> > -
> > org/apache/geode/internal/cache/snapshot/ParallelSnapshotFileMapper,
> > true,1
> >
> >
> > If the class is not persisted or sent over the wire add it to the
> file
> > /tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/
> > resources/org/apache/geode/codeAnalysis/excludedClasses.txt
> > Otherwise if this doesn't break backward compatibility, copy the file
> > /tmp/build/ae3c03f4/geode/geode-core/build/integrationTest/
> actualSerializables.dat
> > to
> > /tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/
> > resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt.
> > at org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest.
> > testSerializables(AnalyzeSerializablesJUnitTest.java:150)
> >
> > 3711 tests completed, 1 failed, 140 skipped
> > :geode-core:integrationTest FAILED
> >
>

[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode-native/pull/115


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: BCSnapshotDUnitTest failing

[Mark Bretl]

Hi Kirk,

There is no 'pivotalgf-assembly' project in Geode. Looks like it is part of
a different distribution. ;)

--Mark

On Fri, Aug 11, 2017 at 4:09 PM, Kirk Lund  wrote:

> BCSnapshotDUnitTest seems to be failing intermittently(?) on develop.
>
> :pivotalgf-assembly:distributedTest
>
> org.apache.geode.BCSnapshotDUnitTest > testSnapshot[0] FAILED
> org.apache.geode.test.dunit.RMIException: While invoking
> org.apache.geode.BCSnapshotDUnitTest$1.run in VM 0 running on Host
> 3dfb24d73711 with 4 VMs
> at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
> at org.apache.geode.test.dunit.VM.invoke(VM.java:302)
> at
> org.apache.geode.BCSnapshotDUnitTest.validateOldSnapshot(
> BCSnapshotDUnitTest.java:103)
> at
> org.apache.geode.BCSnapshotDUnitTest.testSnapshot(
> BCSnapshotDUnitTest.java:95)
>
> Caused by:
> java.io.FileNotFoundException: No snapshot files found in
> /tmp/build/ae3c03f4/gemfire/closed/pivotalgf-assembly/
> build/distributedTest437/diskDir/disk-1/bcCacheSnapshotDir
>
> 14 tests completed, 1 failed, 1 skipped
> :pivotalgf-assembly:distributedTest FAILED
>

[GitHub] geode pull request #709: GEODE-3269: Refactoring ShellCommands

[YehEmily]

GitHub user YehEmily opened a pull request:

https://github.com/apache/geode/pull/709

GEODE-3269: Refactoring ShellCommands

[View the JIRA ticket 
here.](https://issues.apache.org/jira/browse/GEODE-3269)

- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?

- [x] Has your PR been rebased against the latest commit within the target 
branch (typically `develop`)?

- [x] Is your initial contribution a single, squashed commit?

- [x] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes? (See 
[GEODE-1359](https://issues.apache.org/jira/browse/GEODE-1359))

**Testing progress: Precheckin in progress! See eyeh-geode-3269 because I 
probably won't be able to**

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/YehEmily/geode GEODE-3269

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/709.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #709


commit 8a06b768b6e928ab3df6ec1ec4c6460141992bec
Author: YehEmily 
Date:   2017-08-11T23:17:42Z

GEODE-3269: Refactoring ShellCommands




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [DISCUSS] New annotation to identify Functions whose class hierarchy spans multiple jar files

[Kirk Lund]

I can't even say "RegisterableFunction" flibble flobble ;)

On Fri, Aug 11, 2017 at 4:09 PM, Jared Stewart  wrote:

> Hi John,
>
> Thanks for the suggestions. I like “@RegisterableFunction” better than
> “@RegisterFunction".  I think that we don’t want @RegisterableFunction to
> be @Inherited, in order to avoid creating a new variant of the problem we
> are trying to fix.  As you suggest, we should be mindful to document the
> behavior clearly.
>
> In case you’re interested, here’s  fast-classpath-scanner/wiki/3.4.-Finding-classes-with-
> specific-annotations-or-meta-annotations> some documentation of the API
> for the library we’re using for the annotation scanning.  It scans the
> binary byte code files directly, rather than using a Class reference like
> Spring utilities you pointed out (which AFAIK requires having that class
> loaded in the JVM).
>
> Thanks,
> Jared
>
> > On Aug 11, 2017, at 11:00 AM, John Blum  wrote:
> >
> > Hi Jared-
> >
> > In general, I like this idea since Annotations are a great form of
> > meta-data and essentially meaningless outside of the intended context and
> > therefore do not impose any adverse effects on any existing behavior.
> >
> > However, 2 things... 1 suggestion and 1 caution...
> >
> >
> > 1. Perhaps "RegisterableFunction" as opposed to "RegisterFunction".
> >
> >
> > 2. Annotations can be "inherited" in the same way that your
> ConcreteFunction
> > extends (inherits from) AbstractFunction, so too can a more concrete
> > Function possibly inherit from a less-concrete-but-not-abstract Function.
> > A developer might expect that they don't have to re-annotated his/her
> > function.
> >
> > For example...
> >
> > @RegisterableFunction
> > class ProcessOrder extends FunctionAdapter { ... }
> >
> > class CreateAccountAtPointOfSale extends ProcessOrder { ... }
> >
> > If these are in separate JAR files, depending on the "application", I may
> > only want to "register" the CreateAccountAtPointOfSale Function and not
> the
> > ProcessOrder Function explicitly.  Please forgive my example here since
> it
> > seems like a bad example given it feels like 2 separate actions but is
> > often part of the same workflow and so really depends on how application
> > developers think about and organize their logic, which usually leads to
> > "unexpected" things you never anticipated when introducing something like
> > this.
> >
> > SIDE NOTE: Of course, from a Java SE standpoint, the "RegisteredFunction"
> > Annotation could be (but does not have to be) meta-annotated with
> @Inherited.,
> > such as...
> >
> > @Target(ElementType.TYPE)
> > @Retention(RetentionPolicy.RUNTIME)
> > @Inherited
> > ...
> > @interface RegisteredFunction { .. }
> >
> >
> > *Spring* takes all of these things into account when it processes
> > annotations of this nature (e.g. @Enable...).  Especially have a look at
> > o.s.core.annotation.AnnotatedElementUtils [1] and even
> > o.s.core.annotation.AnnotationUtils [2].  These are very robust and very
> > powerful classes underpinning much of *Spring's* Annotation configuration
> > and processing.  *Boot* also extends this functionality and *Spring*
> > Annotation config in very specific/custom ways.
> >
> > I think it is reasonable to set limitations to keep the initial scope
> > small, but be sure those are well documented since users will be coming
> > from many different frameworks having many different expectations.
> >
> > Food for thought/hope this helps.
> >
> > Regards,
> > -John
> >
> >
> > [1]
> > http://docs.spring.io/spring/docs/current/javadoc-api/org/
> springframework/core/annotation/AnnotatedElementUtils.html
> > [2]
> > http://docs.spring.io/spring/docs/current/javadoc-api/org/
> springframework/core/annotation/AnnotationUtils.html
> >
> >
> >
> >
> >
> > On Fri, Aug 11, 2017 at 10:22 AM, Jared Stewart 
> wrote:
> >
> >> Recent changes introduced to avoid the eager loading of classes have
> lead
> >> to Functions not getting registered correctly if the class hierarchy
> >> leading up to Function (or FunctionAdapter) is split up across multiple
> jar
> >> files.  We propose to introduce a new annotation to identify functions
> in
> >> such a case.
> >> Consider the following scenario:
> >>> Abstract.jar - public abstract class AbstractFunction implements
> >> Function {...}
> >>> Concrete.jar - public class ConcreteFunction extends AbstractFunction
> >> {...}
> >> When Concrete.jar is deployed, we only scan the classes inside
> >> Concrete.jar.  This means that we have no way of knowing that
> >> AbstractFunction eventually leads up to Function.  (We could load
> >> ConcreteFunction to see if it implements Function via reflection or
> >> Function.class.isAssignableFrom(), but then we would be back to eagerly
> >> loading all of the classes to see whether or not they implement
> Function.)
> >> We propose a new annotation (perhaps @RegisterFunction, 

Re: [DISCUSS] New annotation to identify Functions whose class hierarchy spans multiple jar files

[Jared Stewart]

Hi John,

Thanks for the suggestions. I like “@RegisterableFunction” better than 
“@RegisterFunction".  I think that we don’t want @RegisterableFunction to be 
@Inherited, in order to avoid creating a new variant of the problem we are 
trying to fix.  As you suggest, we should be mindful to document the behavior 
clearly. 

In case you’re interested, here’s 

 some documentation of the API for the library we’re using for the annotation 
scanning.  It scans the binary byte code files directly, rather than using a 
Class reference like Spring utilities you pointed out (which AFAIK requires 
having that class loaded in the JVM).

Thanks,
Jared

> On Aug 11, 2017, at 11:00 AM, John Blum  wrote:
> 
> Hi Jared-
> 
> In general, I like this idea since Annotations are a great form of
> meta-data and essentially meaningless outside of the intended context and
> therefore do not impose any adverse effects on any existing behavior.
> 
> However, 2 things... 1 suggestion and 1 caution...
> 
> 
> 1. Perhaps "RegisterableFunction" as opposed to "RegisterFunction".
> 
> 
> 2. Annotations can be "inherited" in the same way that your ConcreteFunction
> extends (inherits from) AbstractFunction, so too can a more concrete
> Function possibly inherit from a less-concrete-but-not-abstract Function.
> A developer might expect that they don't have to re-annotated his/her
> function.
> 
> For example...
> 
> @RegisterableFunction
> class ProcessOrder extends FunctionAdapter { ... }
> 
> class CreateAccountAtPointOfSale extends ProcessOrder { ... }
> 
> If these are in separate JAR files, depending on the "application", I may
> only want to "register" the CreateAccountAtPointOfSale Function and not the
> ProcessOrder Function explicitly.  Please forgive my example here since it
> seems like a bad example given it feels like 2 separate actions but is
> often part of the same workflow and so really depends on how application
> developers think about and organize their logic, which usually leads to
> "unexpected" things you never anticipated when introducing something like
> this.
> 
> SIDE NOTE: Of course, from a Java SE standpoint, the "RegisteredFunction"
> Annotation could be (but does not have to be) meta-annotated with @Inherited.,
> such as...
> 
> @Target(ElementType.TYPE)
> @Retention(RetentionPolicy.RUNTIME)
> @Inherited
> ...
> @interface RegisteredFunction { .. }
> 
> 
> *Spring* takes all of these things into account when it processes
> annotations of this nature (e.g. @Enable...).  Especially have a look at
> o.s.core.annotation.AnnotatedElementUtils [1] and even
> o.s.core.annotation.AnnotationUtils [2].  These are very robust and very
> powerful classes underpinning much of *Spring's* Annotation configuration
> and processing.  *Boot* also extends this functionality and *Spring*
> Annotation config in very specific/custom ways.
> 
> I think it is reasonable to set limitations to keep the initial scope
> small, but be sure those are well documented since users will be coming
> from many different frameworks having many different expectations.
> 
> Food for thought/hope this helps.
> 
> Regards,
> -John
> 
> 
> [1]
> http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/core/annotation/AnnotatedElementUtils.html
> [2]
> http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/core/annotation/AnnotationUtils.html
> 
> 
> 
> 
> 
> On Fri, Aug 11, 2017 at 10:22 AM, Jared Stewart  wrote:
> 
>> Recent changes introduced to avoid the eager loading of classes have lead
>> to Functions not getting registered correctly if the class hierarchy
>> leading up to Function (or FunctionAdapter) is split up across multiple jar
>> files.  We propose to introduce a new annotation to identify functions in
>> such a case.
>> Consider the following scenario:
>>> Abstract.jar - public abstract class AbstractFunction implements
>> Function {...}
>>> Concrete.jar - public class ConcreteFunction extends AbstractFunction
>> {...}
>> When Concrete.jar is deployed, we only scan the classes inside
>> Concrete.jar.  This means that we have no way of knowing that
>> AbstractFunction eventually leads up to Function.  (We could load
>> ConcreteFunction to see if it implements Function via reflection or
>> Function.class.isAssignableFrom(), but then we would be back to eagerly
>> loading all of the classes to see whether or not they implement Function.)
>> We propose a new annotation (perhaps @RegisterFunction, suggestions are
>> welcome) to designate a class as a Function in such a case.  Since we are
>> able to scan classes for annotations without loading them, this will allow
>> us to identify ConcreteFunction as a Function without eagerly loading all
>> of the classes in Concrete.jar.
>> I should emphasize that ConcreteFunction already is registered as 

BCSnapshotDUnitTest failing

[Kirk Lund]

BCSnapshotDUnitTest seems to be failing intermittently(?) on develop.

:pivotalgf-assembly:distributedTest

org.apache.geode.BCSnapshotDUnitTest > testSnapshot[0] FAILED
org.apache.geode.test.dunit.RMIException: While invoking
org.apache.geode.BCSnapshotDUnitTest$1.run in VM 0 running on Host
3dfb24d73711 with 4 VMs
at org.apache.geode.test.dunit.VM.invoke(VM.java:387)
at org.apache.geode.test.dunit.VM.invoke(VM.java:357)
at org.apache.geode.test.dunit.VM.invoke(VM.java:302)
at
org.apache.geode.BCSnapshotDUnitTest.validateOldSnapshot(BCSnapshotDUnitTest.java:103)
at
org.apache.geode.BCSnapshotDUnitTest.testSnapshot(BCSnapshotDUnitTest.java:95)

Caused by:
java.io.FileNotFoundException: No snapshot files found in
/tmp/build/ae3c03f4/gemfire/closed/pivotalgf-assembly/build/distributedTest437/diskDir/disk-1/bcCacheSnapshotDir

14 tests completed, 1 failed, 1 skipped
:pivotalgf-assembly:distributedTest FAILED

[GitHub] geode pull request #692: GEODE-3264: Refactoring MemberCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/692


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #665: GEODE-3254: Refactoring ConfigCommands and ConfigCo...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/665


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #690: GEODE-3262: Refactoring IndexCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/690


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #691: GEODE-3260: Refactoring FunctionCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/691


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 61599: GEODE-3328: fix a test failure on windows.

[Jared Stewart]

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


Ship it!




Ship It!

- Jared Stewart


On Aug. 11, 2017, 10:42 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61599/
> ---
> 
> (Updated Aug. 11, 2017, 10:42 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3328: fix a test failure on windows.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
>  da60c7aa481954be0acc8c7e2b88717cf8bc9c37 
> 
> 
> Diff: https://reviews.apache.org/r/61599/diff/1/
> 
> 
> Testing
> ---
> 
> the test itself since only this test is changed.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61599: GEODE-3328: fix a test failure on windows.

[Ken Howe]

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


Ship it!




Replacing hardcoded path strings with system independant constructs is always 
good. I didn't try this out myself on a Windows machine but the fix looks good.

- Ken Howe


On Aug. 11, 2017, 10:42 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61599/
> ---
> 
> (Updated Aug. 11, 2017, 10:42 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-3328: fix a test failure on windows.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
>  da60c7aa481954be0acc8c7e2b88717cf8bc9c37 
> 
> 
> Diff: https://reviews.apache.org/r/61599/diff/1/
> 
> 
> Testing
> ---
> 
> the test itself since only this test is changed.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #644 was SUCCESSFUL (with 2027 tests)

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #644 was successful.
---
Scheduled
2029 tests in total.

https://build.spring.io/browse/SGF-NAG-644/





--
This message is automatically generated by Atlassian Bamboo

Review Request 61599: GEODE-3328: fix a test failure on windows.

[Jinmei Liao]

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

Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

GEODE-3328: fix a test failure on windows.


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GfshCommandJUnitTest.java
 da60c7aa481954be0acc8c7e2b88717cf8bc9c37 


Diff: https://reviews.apache.org/r/61599/diff/1/


Testing
---

the test itself since only this test is changed.


Thanks,

Jinmei Liao

Re: ParallelSnapshotFileMapper broke AnalyzeSerializablesJUnitTest

[Kirk Lund]

Looks like it was this commit...

commit 7072f8ef7b764d1507e26cca8ed0c4d184ccc81a
Author: Nick Reich 
Date:   Fri Jul 28 16:47:10 2017 -0700

GEODE-3300: Complete and expose parallel export feature for use

This closes #704


On Fri, Aug 11, 2017 at 3:40 PM, Kirk Lund  wrote:

> Looks like one of the recent commits to develop resulted in this failure
> involving org/apache/geode/internal/cache/snapshot/
> ParallelSnapshotFileMapper...
>
> :geode-core:integrationTest
>
> org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest >
> testSerializables FAILED
> java.lang.AssertionError: New or moved classes---
> -
> org/apache/geode/internal/cache/snapshot/ParallelSnapshotFileMapper,
> true,1
>
>
> If the class is not persisted or sent over the wire add it to the file
> /tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/
> resources/org/apache/geode/codeAnalysis/excludedClasses.txt
> Otherwise if this doesn't break backward compatibility, copy the file
> 
> /tmp/build/ae3c03f4/geode/geode-core/build/integrationTest/actualSerializables.dat
> to
> /tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/
> resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt.
> at org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest.
> testSerializables(AnalyzeSerializablesJUnitTest.java:150)
>
> 3711 tests completed, 1 failed, 140 skipped
> :geode-core:integrationTest FAILED
>

ParallelSnapshotFileMapper broke AnalyzeSerializablesJUnitTest

[Kirk Lund]

Looks like one of the recent commits to develop resulted in this failure
involving
org/apache/geode/internal/cache/snapshot/ParallelSnapshotFileMapper...

:geode-core:integrationTest

org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest >
testSerializables FAILED
java.lang.AssertionError: New or moved
classes

org/apache/geode/internal/cache/snapshot/ParallelSnapshotFileMapper,true,1


If the class is not persisted or sent over the wire add it to the file

/tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
Otherwise if this doesn't break backward compatibility, copy the file

/tmp/build/ae3c03f4/geode/geode-core/build/integrationTest/actualSerializables.dat
to

/tmp/build/ae3c03f4/gemfire/open/geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt.
at
org.apache.geode.codeAnalysis.AnalyzeSerializablesJUnitTest.testSerializables(AnalyzeSerializablesJUnitTest.java:150)

3711 tests completed, 1 failed, 140 skipped
:geode-core:integrationTest FAILED

[GitHub] geode pull request #687: GEODE-3258: Refactoring DiskStoreCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/687


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #701: GEODE-3337: Refactoring LauncherLifecycleCommandsDU...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/701


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #696: GEODE-3265: Refactoring MiscellaneousCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/696


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #685: GEODE-3261: Refactoring GfshHelpCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/685


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 61563: GEODE-3383: Refactor deploy tests

[Jared Stewart]

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

(Updated Aug. 11, 2017, 9:52 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description
---

GEODE-3383: Refactor deploy tests

- Refactor DeployedJarJUnitTest
- Move several tests into more appropriate locations


Diffs
-

  
geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderIntegrationTest.java
 34d8a2318422edbb3bbdc04920a41693f8d3fe69 
  geode-core/src/test/java/org/apache/geode/internal/DeployedJarJUnitTest.java 
178dbae2eaada0cc054502fdf4b6d2ff102b4ed6 
  
geode-core/src/test/java/org/apache/geode/internal/JarDeployerDeadlockTest.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/DeployJarTestSuite.java 
6dfab66926c7b246bf839632b293330959f1d728 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
 89148d7752ae1f69e25671ffc43101a63cf7dc64 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java
 7753aafbd7dc5ea4288e27f088400163f6739347 


Diff: https://reviews.apache.org/r/61563/diff/1/


Testing (updated)
---

Precheckin passed


Thanks,

Jared Stewart

[GitHub] geode pull request #695: GEODE-3267: Refactoring QueueCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/695


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #694: GEODE-3270: Refactoring (renaming) StatusCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/694


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #693: GEODE-3268: Refactoring RegionCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/693


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #114: GEODE-2729: Remove global variables

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/114
  
This was merged without auto-close comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #684: GEODE-3266: Refactoring PDXCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/684


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #680: GEODE-3257: Refactoring DeployCommands

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/680


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Geode Native - All your globals are belong to us

[Udo Kohlmeyer]

Awesome work!!!

On 8/11/17 13:23, Jacob Barrett wrote:

In case you missed the big commit recently, the Geode Native components has
removed all globals.*

The biggest win here is that we can more easily unit test sections of the
code that before was nearly impossible due to reliance on initialized
global instances of Cache, DistributedSystem, PoolManager, etc. All long
lived instances are children of Cache and have access to their "container"
Cache for accessing other components like PoolManager, DistributedSystem,
etc. The refactor probably isn't perfect but it is a step in the right
direction. Look for more unit tests to come.

The next big win is that you can truly have more than a single Cache
instance in our process space. While this isn't a common intentional use
case, it is common in the .NET client implementation since a single
process, AppPool, could have several AppDomains each with their own
instance of Cache. In the old model all managed Cache instances shared the
singleton unmanaged Cache instance. This lead to confusion around
configuring multiple Caches in the same AppPool, think session state
provider and application lookaside caching in IIS, and restricted
configuration to share the same cluster and configuration.

Obviously this is a HUGE breaking change. Almost all source using the C++
or .NET clients will need to be refactored. While many should be pretty
obvious, look for updates coming to the examples soon to show you the light.

This change follows the major change where we switch from the home grown
SharedPtr/SharedBase refcounting memory management model to
std::shared_ptr. The next major refactoring coming is aligning the public
API with C++11 standards, like std::string, range-based for loops, etc. We
also plan to make the API more consistent between factories, getters,
setters, refs, pointers, etc.

-Jake

* Some lingering globals have been identified for removal.

Re: Geode Native - All your globals are belong to us

[Kirk Lund]

Congratulations and thank you!

On Fri, Aug 11, 2017 at 1:23 PM, Jacob Barrett  wrote:

> In case you missed the big commit recently, the Geode Native components has
> removed all globals.*
>
> The biggest win here is that we can more easily unit test sections of the
> code that before was nearly impossible due to reliance on initialized
> global instances of Cache, DistributedSystem, PoolManager, etc. All long
> lived instances are children of Cache and have access to their "container"
> Cache for accessing other components like PoolManager, DistributedSystem,
> etc. The refactor probably isn't perfect but it is a step in the right
> direction. Look for more unit tests to come.
>
> The next big win is that you can truly have more than a single Cache
> instance in our process space. While this isn't a common intentional use
> case, it is common in the .NET client implementation since a single
> process, AppPool, could have several AppDomains each with their own
> instance of Cache. In the old model all managed Cache instances shared the
> singleton unmanaged Cache instance. This lead to confusion around
> configuring multiple Caches in the same AppPool, think session state
> provider and application lookaside caching in IIS, and restricted
> configuration to share the same cluster and configuration.
>
> Obviously this is a HUGE breaking change. Almost all source using the C++
> or .NET clients will need to be refactored. While many should be pretty
> obvious, look for updates coming to the examples soon to show you the
> light.
>
> This change follows the major change where we switch from the home grown
> SharedPtr/SharedBase refcounting memory management model to
> std::shared_ptr. The next major refactoring coming is aligning the public
> API with C++11 standards, like std::string, range-based for loops, etc. We
> also plan to make the API more consistent between factories, getters,
> setters, refs, pointers, etc.
>
> -Jake
>
> * Some lingering globals have been identified for removal.
>

[GitHub] geode issue #672: GEODE-3256: Refactoring DataCommands

[YehEmily]

Github user YehEmily commented on the issue:

https://github.com/apache/geode/pull/672
  
Already merged to develop.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Geode Native - All your globals are belong to us

[Jacob Barrett]

In case you missed the big commit recently, the Geode Native components has
removed all globals.*

The biggest win here is that we can more easily unit test sections of the
code that before was nearly impossible due to reliance on initialized
global instances of Cache, DistributedSystem, PoolManager, etc. All long
lived instances are children of Cache and have access to their "container"
Cache for accessing other components like PoolManager, DistributedSystem,
etc. The refactor probably isn't perfect but it is a step in the right
direction. Look for more unit tests to come.

The next big win is that you can truly have more than a single Cache
instance in our process space. While this isn't a common intentional use
case, it is common in the .NET client implementation since a single
process, AppPool, could have several AppDomains each with their own
instance of Cache. In the old model all managed Cache instances shared the
singleton unmanaged Cache instance. This lead to confusion around
configuring multiple Caches in the same AppPool, think session state
provider and application lookaside caching in IIS, and restricted
configuration to share the same cluster and configuration.

Obviously this is a HUGE breaking change. Almost all source using the C++
or .NET clients will need to be refactored. While many should be pretty
obvious, look for updates coming to the examples soon to show you the light.

This change follows the major change where we switch from the home grown
SharedPtr/SharedBase refcounting memory management model to
std::shared_ptr. The next major refactoring coming is aligning the public
API with C++11 standards, like std::string, range-based for loops, etc. We
also plan to make the API more consistent between factories, getters,
setters, refs, pointers, etc.

-Jake

* Some lingering globals have been identified for removal.

[GitHub] geode pull request #672: GEODE-3256: Refactoring DataCommands

[YehEmily]

Github user YehEmily closed the pull request at:

https://github.com/apache/geode/pull/672


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...

[dgkimura]

Github user dgkimura commented on the issue:

https://github.com/apache/geode-native/pull/115
  
@pivotal-jbarrett: Ship-it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #671: GEODE-3255: Refactor CreateAlterDestroyRegionComman...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/671


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/115
  
@dgkimura https://issues.apache.org/jira/browse/GEODE-3432

Anything else you don't like?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/115
  
@dgkimura Those files actually need to be redone in a way more closely 
aligned with other CMake find modules. I would leave them alone for now and fix 
later.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #651: GEODE-3230: Cleaning up unused (Cli)Strings

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/651


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/115#discussion_r132769880
  
--- Diff: CMakeLists.txt ---
@@ -242,7 +242,7 @@ add_subdirectory(dhimpl)
 add_subdirectory(sqliteimpl)
 add_subdirectory(tests)
 add_subdirectory(templates/security)
-add_subdirectory(docs)
+add_subdirectory(docs/api)
--- End diff --

There are API docs and the manual.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #699: GEODE-3413: overhaul launcher and process classes a...

[kirklund]

Github user kirklund closed the pull request at:

https://github.com/apache/geode/pull/699


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #699: GEODE-3413: overhaul launcher and process classes and test...

[kirklund]

Github user kirklund commented on the issue:

https://github.com/apache/geode/pull/699
  
Merged this PR to develop. Done!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #704: GEODE-3300: Complete and expose parallel export fea...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/704


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-examples pull request #13: GEODE-3428: Add example of putting multiple...

[PivotalSarge]

GitHub user PivotalSarge opened a pull request:

https://github.com/apache/geode-examples/pull/13

GEODE-3428: Add example of putting multiple values all at once.

Add an example of `Region.putAll()`.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/PivotalSarge/geode-examples feature/GEODE-3428

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode-examples/pull/13.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #13


commit 50852add495930cc7d310efff191d72fda35f5c7
Author: Sarge 
Date:   2017-08-11T18:50:53Z

GEODE-3428: Add example of putting multiple values all at once.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: []DISCUSS] Using package names to identify public API's

[Ernest Burghardt]

+1 for protobuf being internal as they are not intended to be first class
OO citizens you can read more about there here
 (the
same warning exists for all supported languages btw)

On Fri, Aug 11, 2017 at 12:34 PM, Michael William Dodge 
wrote:

> The user shouldn't need to access any of the protobuf classes directly.
> I'm in favor of making all of the protobuf-related packages internal,
> including any classes generated from .proto files.
>
> Sarge
>
> > On 11 Aug, 2017, at 11:30, Anthony Baker  wrote:
> >
> > We have policies in place for versioning [1] and backwards compatibility
> [2].  How do we identify which API’s need to be controlled?
> >
> > In many cases we use the *.internal.* package naming format to signal
> API’s that aren’t subject to backwards compatibility requirements.  API’s
> within these internal packages can change and do change even within minor
> or patch releases.  If a user creates an application that relies on an
> internal API, it may need to be changed during an upgrade.
> >
> > I’ve noticed that we haven’t been following this convention for some
> newer changes (such as in geode-protobuf).  Should we review and modify the
> packages names continue using the *.internal.* format?
> >
> >
> > Anthony
> >
> > [1] https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=57311457
> > [1] https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+
> Compatibility
> >
>
>

[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...

[dgkimura]

Github user dgkimura commented on the issue:

https://github.com/apache/geode-native/pull/115
  
@pivotal-jbarrett I was referring to `FindNativeClient.cmake` and 
`FindNativeClientCPPCache.cmake`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...

[dgkimura]

Github user dgkimura commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/115#discussion_r132757951
  
--- Diff: CMakeLists.txt ---
@@ -242,7 +242,7 @@ add_subdirectory(dhimpl)
 add_subdirectory(sqliteimpl)
 add_subdirectory(tests)
 add_subdirectory(templates/security)
-add_subdirectory(docs)
+add_subdirectory(docs/api)
--- End diff --

Ah, I see. So we have 2 docs? That's a little confusing..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: []DISCUSS] Using package names to identify public API's

[Kirk Lund]

One nice thing about releasing new code in *internal* packages is that we
can always move them out of the *internal* packages at a later date but we
cannot move non-deprecated code from external API packages into *internal*
packages without worrying about backwards compatibility.

On Fri, Aug 11, 2017 at 11:34 AM, Michael William Dodge 
wrote:

> The user shouldn't need to access any of the protobuf classes directly.
> I'm in favor of making all of the protobuf-related packages internal,
> including any classes generated from .proto files.
>
> Sarge
>
> > On 11 Aug, 2017, at 11:30, Anthony Baker  wrote:
> >
> > We have policies in place for versioning [1] and backwards compatibility
> [2].  How do we identify which API’s need to be controlled?
> >
> > In many cases we use the *.internal.* package naming format to signal
> API’s that aren’t subject to backwards compatibility requirements.  API’s
> within these internal packages can change and do change even within minor
> or patch releases.  If a user creates an application that relies on an
> internal API, it may need to be changed during an upgrade.
> >
> > I’ve noticed that we haven’t been following this convention for some
> newer changes (such as in geode-protobuf).  Should we review and modify the
> packages names continue using the *.internal.* format?
> >
> >
> > Anthony
> >
> > [1] https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=57311457
> > [1] https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+
> Compatibility
> >
>
>

Re: []DISCUSS] Using package names to identify public API's

[Kirk Lund]

+1 to continue using *internal* to differentiate code from external APIs

The alternative would require Geode to be more modular, in which we have
the external API in API- or SPI-specific jars and then the "internal"
packages go into an implementation jar that needs to be on the classpath
during runtime.

On Fri, Aug 11, 2017 at 11:30 AM, Anthony Baker  wrote:

> We have policies in place for versioning [1] and backwards compatibility
> [2].  How do we identify which API’s need to be controlled?
>
> In many cases we use the *.internal.* package naming format to signal
> API’s that aren’t subject to backwards compatibility requirements.  API’s
> within these internal packages can change and do change even within minor
> or patch releases.  If a user creates an application that relies on an
> internal API, it may need to be changed during an upgrade.
>
> I’ve noticed that we haven’t been following this convention for some newer
> changes (such as in geode-protobuf).  Should we review and modify the
> packages names continue using the *.internal.* format?
>
>
> Anthony
>
> [1] https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=57311457
> [1] https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+
> Compatibility
>
>

Re: []DISCUSS] Using package names to identify public API's

[Michael William Dodge]

The user shouldn't need to access any of the protobuf classes directly. I'm in 
favor of making all of the protobuf-related packages internal, including any 
classes generated from .proto files.

Sarge
 
> On 11 Aug, 2017, at 11:30, Anthony Baker  wrote:
> 
> We have policies in place for versioning [1] and backwards compatibility [2]. 
>  How do we identify which API’s need to be controlled?
> 
> In many cases we use the *.internal.* package naming format to signal API’s 
> that aren’t subject to backwards compatibility requirements.  API’s within 
> these internal packages can change and do change even within minor or patch 
> releases.  If a user creates an application that relies on an internal API, 
> it may need to be changed during an upgrade.
> 
> I’ve noticed that we haven’t been following this convention for some newer 
> changes (such as in geode-protobuf).  Should we review and modify the 
> packages names continue using the *.internal.* format?
> 
> 
> Anthony
> 
> [1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=57311457
> [1] 
> https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+Compatibility
> 

[GitHub] geode-native issue #115: GEODE-3165: Reogranized sources relative to the roo...

[pivotal-jbarrett]

Github user pivotal-jbarrett commented on the issue:

https://github.com/apache/geode-native/pull/115
  
@dgkimura which CMake find modules are you referring to?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[]DISCUSS] Using package names to identify public API's

[Anthony Baker]

We have policies in place for versioning [1] and backwards compatibility [2].  
How do we identify which API’s need to be controlled?

In many cases we use the *.internal.* package naming format to signal API’s 
that aren’t subject to backwards compatibility requirements.  API’s within 
these internal packages can change and do change even within minor or patch 
releases.  If a user creates an application that relies on an internal API, it 
may need to be changed during an upgrade.

I’ve noticed that we haven’t been following this convention for some newer 
changes (such as in geode-protobuf).  Should we review and modify the packages 
names continue using the *.internal.* format?


Anthony

[1] https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=57311457
[1] 
https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+Compatibility

Re: [DISCUSS] New annotation to identify Functions whose class hierarchy spans multiple jar files

[John Blum]

Hi Jared-

In general, I like this idea since Annotations are a great form of
meta-data and essentially meaningless outside of the intended context and
therefore do not impose any adverse effects on any existing behavior.

However, 2 things... 1 suggestion and 1 caution...


1. Perhaps "RegisterableFunction" as opposed to "RegisterFunction".


2. Annotations can be "inherited" in the same way that your ConcreteFunction
extends (inherits from) AbstractFunction, so too can a more concrete
Function possibly inherit from a less-concrete-but-not-abstract Function.
A developer might expect that they don't have to re-annotated his/her
function.

For example...

@RegisterableFunction
class ProcessOrder extends FunctionAdapter { ... }

class CreateAccountAtPointOfSale extends ProcessOrder { ... }

If these are in separate JAR files, depending on the "application", I may
only want to "register" the CreateAccountAtPointOfSale Function and not the
ProcessOrder Function explicitly.  Please forgive my example here since it
seems like a bad example given it feels like 2 separate actions but is
often part of the same workflow and so really depends on how application
developers think about and organize their logic, which usually leads to
"unexpected" things you never anticipated when introducing something like
this.

SIDE NOTE: Of course, from a Java SE standpoint, the "RegisteredFunction"
Annotation could be (but does not have to be) meta-annotated with @Inherited.,
such as...

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Inherited
...
@interface RegisteredFunction { .. }


*Spring* takes all of these things into account when it processes
annotations of this nature (e.g. @Enable...).  Especially have a look at
o.s.core.annotation.AnnotatedElementUtils [1] and even
o.s.core.annotation.AnnotationUtils [2].  These are very robust and very
powerful classes underpinning much of *Spring's* Annotation configuration
and processing.  *Boot* also extends this functionality and *Spring*
Annotation config in very specific/custom ways.

I think it is reasonable to set limitations to keep the initial scope
small, but be sure those are well documented since users will be coming
from many different frameworks having many different expectations.

Food for thought/hope this helps.

Regards,
-John


[1]
http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/core/annotation/AnnotatedElementUtils.html
[2]
http://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/core/annotation/AnnotationUtils.html





On Fri, Aug 11, 2017 at 10:22 AM, Jared Stewart  wrote:

> Recent changes introduced to avoid the eager loading of classes have lead
> to Functions not getting registered correctly if the class hierarchy
> leading up to Function (or FunctionAdapter) is split up across multiple jar
> files.  We propose to introduce a new annotation to identify functions in
> such a case.
> Consider the following scenario:
> > Abstract.jar - public abstract class AbstractFunction implements
> Function {...}
> > Concrete.jar - public class ConcreteFunction extends AbstractFunction
> {...}
> When Concrete.jar is deployed, we only scan the classes inside
> Concrete.jar.  This means that we have no way of knowing that
> AbstractFunction eventually leads up to Function.  (We could load
> ConcreteFunction to see if it implements Function via reflection or
> Function.class.isAssignableFrom(), but then we would be back to eagerly
> loading all of the classes to see whether or not they implement Function.)
> We propose a new annotation (perhaps @RegisterFunction, suggestions are
> welcome) to designate a class as a Function in such a case.  Since we are
> able to scan classes for annotations without loading them, this will allow
> us to identify ConcreteFunction as a Function without eagerly loading all
> of the classes in Concrete.jar.
> I should emphasize that ConcreteFunction already is registered as expected
> if it resides in the same jar file as AbstractFunction.  This annotation
> would only be relevant when the class hierarchy leading up to Function is
> spread across multiple jar files.
> - Jared




-- 
-John
john.blum10101 (skype)

[GitHub] geode pull request #708: GEODE-3410 Doc update for gfsh query command change...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/708


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132735527
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
+  if (authenticatorClass != null) {
+return authenticatorClass;
+  }
+
+  ServiceLoader loader = 
ServiceLoader.load(StreamAuthenticator.class);
+
+  for (StreamAuthenticator classInstance : loader) {
+if (implementationID.equals(classInstance.implementationID())) {
+  return classInstance.getClass();
--- End diff --

Why do we get an instance of the StreamAuthenticator, just to call 
`getClass` on it, and then later we create a new instance of class from this 
instance. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132735728
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -72,9 +99,15 @@ public static ServerConnection 
makeServerConnection(Socket s, InternalCache c,
 throw new IOException("Acceptor received unknown communication 
mode: " + communicationMode);
   } else {
 protobufProtocolHandler = findClientProtocolMessageHandler();
-return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout, socketBufferSize,
-communicationModeStr, communicationMode, acceptor, 
protobufProtocolHandler,
-securityService);
+authenticatorClass = findStreamAuthenticator(
+
c.getInternalDistributedSystem().getConfig().getProtobufProtocolAuthenticationMode());
+try {
+  return new GenericProtocolServerConnection(s, c, helper, stats, 
hsTimeout,
+  socketBufferSize, communicationModeStr, communicationMode, 
acceptor,
+  protobufProtocolHandler, securityService, 
authenticatorClass.newInstance());
--- End diff --

We should use the authenticator instance returned from the 
`findClientProtocolMessageHandler`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132737475
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
 ---
@@ -2433,6 +2433,25 @@
   String getRedundancyZone();
 
   /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttribute(type = String.class)
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE_NAME = 
PROTOBUF_PROTOCOL_AUTHENTICATION_MODE;
+  String DEFAULT_PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = "NOOP";
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeSetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  void setProtobufProtocolAuthenticationMode(String authenticationMode);
+
+  /**
+   * @since Geode 1.??? TODO FIXME
+   */
+  @ConfigAttributeGetter(name = PROTOBUF_PROTOCOL_AUTHENTICATION_MODE)
+  String getProtobufProtocolAuthenticationMode();
+
+  /**
--- End diff --

Without an official release version I don't think we should have any of the 
properties/configurations public until release.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132737182
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 ---
@@ -1378,6 +1379,18 @@
*/
   String NAME = "name";
   /**
+   * The authentication mode for the protobuf client-server protocol.
+   *
+   * 
+   * Description: Specifies the authentication mode used by the 
geode-protobuf module.
+   * 
+   * Default: "NOOP"
+   * 
+   * Allowed values: "NOOP" "SIMPLE"
+   */
+  @Experimental
+  String PROTOBUF_PROTOCOL_AUTHENTICATION_MODE = 
"protobuf-protocol-authentication-mode";
--- End diff --

This property is misleading. It is NOT a protobuf specific authentication 
mode. It is merely an authentication mechanism that uses protobuf underneath 
the covers.
1) A different property name is to be used
2) With this property, the feature toggle should also maybe be removed??!!? 
One cannot live without the other


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132738771
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnectionFactory.java
 ---
@@ -63,6 +65,31 @@ private static ClientProtocolMessageHandler 
findClientProtocolMessageHandler() {
 }
   }
 
+  private static Class 
findStreamAuthenticator(
+  String implementationID) {
+if (authenticatorClass != null) {
+  return authenticatorClass;
+}
+
+synchronized (streamAuthenticatorLoadLock) {
--- End diff --

why do we prefer this approach over a synchronized method?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #707: GEODE-3412: Add simple authentication flow to proto...

[kohlmu-pivotal]

Github user kohlmu-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/707#discussion_r132738100
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/NoOpStreamAuthenticator.java
 ---
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for 
additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF 
ANY KIND, either express
+ * or implied. See the License for the specific language governing 
permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache.tier.sockets;
--- End diff --

I'm not sure this class should live in this package. Is it not more related 
to security?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #115: GEODE-3165: Reogranized sources relative to ...

[pivotal-jbarrett]

GitHub user pivotal-jbarrett opened a pull request:

https://github.com/apache/geode-native/pull/115

GEODE-3165: Reogranized sources relative to the root for better CMake

… IDE integration.

- Moved src/docs to docs/api.
- Moved src/* to root.
- Fixup paths in CMake files.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/pivotal-jbarrett/geode-native 
feature/GEODE-3165

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode-native/pull/115.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #115


commit 6cbd424fe39bc907025460c555768552430c0c5d
Author: Jacob Barrett 
Date:   2017-08-11T15:30:49Z

GEODE-3165: Reogranized sources relative to the root for better CMake IDE 
integration.

- Moved src/docs to docs/api.
- Moved src/* to root.
- Fixup paths in CMake files.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-examples pull request #12: Add OQL example

[PivotalSarge]

Github user PivotalSarge closed the pull request at:

https://github.com/apache/geode-examples/pull/12


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 61562: GEODE-3423: Provide support for running parallel docker builds in Jenkins

[Anthony Baker]

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


Ship it!




Ship It!

- Anthony Baker


On Aug. 10, 2017, 9:16 p.m., Jens Deppe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61562/
> ---
> 
> (Updated Aug. 10, 2017, 9:16 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Mark Bretl.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> - Also cleaning up other Dockerfiles which are unused
> 
> Signed-off-by: Scott Jewell 
> 
> 
> Diffs
> -
> 
>   dev-tools/docker/base/Dockerfile 1cce0ddb0d29c9e188d27481c82123e357c5b685 
>   dev-tools/docker/base/build-base-docker.sh 
> 9aab72c45d63c519874ad39aaf74c8236d9671ed 
>   dev-tools/docker/compile/Dockerfile 
> 6ae343a70eccf7cc4b303228118cee0b8579e79a 
>   dev-tools/docker/compile/start-compile-docker.sh 
> 9059c5b5bbffbadaa82277c05e2189360d94a484 
>   gradle/docker.gradle 79719740bc85fd1939d2d7859a7c78c0a87dd26e 
> 
> 
> Diff: https://reviews.apache.org/r/61562/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jens Deppe
> 
>

Re: Gitbox enables the full GitHub workflow

[Jacob Barrett]

The geode-native synch from ASF to GitHub is stuck again and while we are
waiting for Infra to make the fix it occurred to me it might be more
productive to have them just move us to GitBox. We have another big commit
coming that will likely break the commit again. Does anyone object to
geode-native being the guinea pig for GitBox?

Re: Gitbox enables the full GitHub workflow

[Ernest Burghardt]

+1

On Wed, Aug 9, 2017 at 2:57 PM, Nabarun Nag  wrote:

> +1
>
> will this allow us to choose reviewers while creating PRs on github now?
>
> Regards
> Nabarun
>
> On Wed, Aug 9, 2017 at 1:19 PM Udo Kohlmeyer 
> wrote:
>
> > +1
> >
> >
> > On 8/9/17 12:56, Anthony Baker wrote:
> > > +1
> > >
> > >> On Aug 8, 2017, at 12:46 PM, Mark Bretl  wrote:
> > >>
> > >> +1 for Gitbox
> > >>
> > >> I have been watching threads on INFRA list and been waiting to see
> when
> > a
> > >> good time would be to introduce the idea to the community. The Gitbox
> > >> project has been going since the end of 2016 and looks as though it
> > might
> > >> be ready now as INFRA has begun moving some of their repositories to
> > >> GitHub. We still need to 'apply' by submitting a ticket to INFRA and
> > wait
> > >> for approval, INFRA could still deny the move.
> > >>
> > >> Advantages:
> > >> - GitHub PRs are fully functional (something which stopped us earlier)
> > >> - Provides ability to align committer and non-committer workflows
> > >> - Removing mirror from Git at Apache to GitHub and thus removing delay
> > in
> > >> sync
> > >>
> > >> Disadvantages:
> > >> - Permission mapping is configured on id.apache.org (need to set
> > GitHub ID)
> > >>
> > >> I can't think of many disadvantages to moving to GitBox.
> > >>
> > >> --Mark
> > >>
> > >>
> > >> On Tue, Aug 8, 2017 at 10:56 AM, Dan Smith  wrote:
> > >>
> > >>> I'm in favor of switching to gitbox. We're already getting pretty
> much
> > all
> > >>> contributions from non-committers coming in as pull requests so we
> > might as
> > >>> well make it easier for us to manage things in github.
> > >>>
> > >>> -Dan
> > >>>
> > >>> On Tue, Aug 8, 2017 at 8:57 AM, Kirk Lund  wrote:
> > >>>
> >  One thing that's given me trouble is that the mirroring of ASF git
> > (for
> >  Apache Geode) to github can have a lengthy delay. In other words,
> > after I
> >  commit to ASF git, it's not uncommon for my commit to not show up in
> > the
> >  GitHub mirror right away and I've seen this delay take an hour.
> > 
> >  On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik <
> > ro...@shaposhnik.org>
> >  wrote:
> > 
> > > Hi!
> > >
> > > it has just come to my attention that Gitbox at ASF
> > > has been enabling full GitHub workflow (with being
> > > able to click Merge this PR button, etc.) for quite
> > > some time.
> > >
> > > This basically allows a project to have GH as a R/W
> > > repo as opposed to R/O mirror of what we all currnently
> > > have: https://gitbox.apache.org/repos/asf
> > >
> > > Personally I'm not sure I like GH workflow all that much,
> > > but if there's interest -- you can opt-in into Gitbox. Once
> > > you do -- your source of truth moves to GH. You can't
> > > have it both ways with git-wip-us.apache.org and Gitbox.
> > >
> > > Thanks,
> > > Roman.
> > >
> >
> >
>

Re: Review Request 61540: GEODE-3403: Modify rolling upgrade test configuration for 1.2.x release

[Udo Kohlmeyer]

> On Aug. 10, 2017, 7 p.m., Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/Version.java
> > Line 205 (original), 210 (patched)
> > 
> >
> > what is the correlation between this and HIGHED_VERSION?
> > Could we automate this? External file? etc
> 
> Bruce Schuchardt wrote:
> HIGHEST_VERSION is used to initialize the array of Version instances.  
> This must be done before any Version instances are created.  I think it would 
> be a different ticket, probably a "chore", to rework this class to remove 
> this variable.  I don't intend to do that work as part of this ticket.

Sorry... this was not meant as a "we must address before we release"... more of 
"maybe we can make this easier".


- Udo


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


On Aug. 9, 2017, 8:45 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61540/
> ---
> 
> (Updated Aug. 9, 2017, 8:45 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Udo Kohlmeyer, 
> and Brian Rowe.
> 
> 
> Bugs: GEODE-3403
> https://issues.apache.org/jira/browse/GEODE-3403
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Nabarun has already added a test120 source set in geode-old-versions.  This 
> commit will roll the version on develop to 1.3.0 so that it will have a 
> different version ordinal than test120.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 39e3a3f18d90567a1e3564884760014f6daf1f4c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  9995e4658e5afcb5eb9823913e73aa04d1cdbdbd 
> 
> 
> Diff: https://reviews.apache.org/r/61540/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61540: GEODE-3403: Modify rolling upgrade test configuration for 1.2.x release

[Udo Kohlmeyer]

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Aug. 9, 2017, 8:45 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61540/
> ---
> 
> (Updated Aug. 9, 2017, 8:45 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Udo Kohlmeyer, 
> and Brian Rowe.
> 
> 
> Bugs: GEODE-3403
> https://issues.apache.org/jira/browse/GEODE-3403
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Nabarun has already added a test120 source set in geode-old-versions.  This 
> commit will roll the version on develop to 1.3.0 so that it will have a 
> different version ordinal than test120.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 39e3a3f18d90567a1e3564884760014f6daf1f4c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  9995e4658e5afcb5eb9823913e73aa04d1cdbdbd 
> 
> 
> Diff: https://reviews.apache.org/r/61540/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 61540: GEODE-3403: Modify rolling upgrade test configuration for 1.2.x release

[Bruce Schuchardt]

> On Aug. 10, 2017, noon, Udo Kohlmeyer wrote:
> > geode-core/src/main/java/org/apache/geode/internal/Version.java
> > Line 205 (original), 210 (patched)
> > 
> >
> > what is the correlation between this and HIGHED_VERSION?
> > Could we automate this? External file? etc

HIGHEST_VERSION is used to initialize the array of Version instances.  This 
must be done before any Version instances are created.  I think it would be a 
different ticket, probably a "chore", to rework this class to remove this 
variable.  I don't intend to do that work as part of this ticket.


- Bruce


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


On Aug. 9, 2017, 1:45 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61540/
> ---
> 
> (Updated Aug. 9, 2017, 1:45 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Galen O'Sullivan, Udo Kohlmeyer, 
> and Brian Rowe.
> 
> 
> Bugs: GEODE-3403
> https://issues.apache.org/jira/browse/GEODE-3403
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Nabarun has already added a test120 source set in geode-old-versions.  This 
> commit will roll the version on develop to 1.3.0 so that it will have a 
> different version ordinal than test120.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/Version.java 
> 39e3a3f18d90567a1e3564884760014f6daf1f4c 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/CommandInitializer.java
>  9995e4658e5afcb5eb9823913e73aa04d1cdbdbd 
> 
> 
> Diff: https://reviews.apache.org/r/61540/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

[GitHub] geode issue #467: GEODE-258: Remove deprecated Cache.getLoggerI18n and getSe...

[davinash]

Github user davinash commented on the issue:

https://github.com/apache/geode/pull/467
  
Thanks @kirklund  and @upthewaterspout for your comments. I am moving this 
discussion on  JIRA GEODE-258 and more questions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #609: GEODE-2886 : sent IllegalStateException instead of ...

[ameybarve15]

Github user ameybarve15 commented on a diff in the pull request:

https://github.com/apache/geode/pull/609#discussion_r132678804
  
--- Diff: 
geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
 ---
@@ -331,6 +331,29 @@ public void queryJsonObject() throws Exception {
   }
 
   @Test()
+  public void waitUntilFlushThrowsIllegalStateExceptionWhenAEQNotFound() 
throws Exception {
+Map fields = new HashMap<>();
+fields.put("name", null);
+fields.put("lastName", null);
+fields.put("address", null);
+
luceneService.createIndexFactory().setFields(fields).create(INDEX_NAME, 
REGION_NAME);
+Region region = 
cache.createRegionFactory(RegionShortcut.PARTITION).create(REGION_NAME);
+final LuceneIndex index = luceneService.getIndex(INDEX_NAME, 
REGION_NAME);
+
+// This is to send IllegalStateException from WaitUntilFlushedFunction
+String nonCreatedIndex = "index2";
+boolean result = false;
+try {
+  result = luceneService.waitUntilFlushed(nonCreatedIndex, 
REGION_NAME, 6,
+  TimeUnit.MILLISECONDS);
--- End diff --

Thanks @jhuynh1 
Please review again, I updated test case.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode issue #698: Mark ProtoBuf interface as experimental

[galen-pivotal]

Github user galen-pivotal commented on the issue:

https://github.com/apache/geode/pull/698
  
What you've done looks good. However, it looks like there are ten or so 
files in `geode-protobuf` that you didn't mark as experimental. Is there a 
reason for this?

Have a look at the difference between these:

```bash
find geode-protobuf/src/main/ -name '*.java' 
find geode-protobuf/src/main/ -name '*.java' | xargs grep -l '@Experimental'
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode pull request #682: GEODE-3393: One-way SSL authentication fails with F...

[galen-pivotal]

Github user galen-pivotal commented on a diff in the pull request:

https://github.com/apache/geode/pull/682#discussion_r132626571
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java ---
@@ -33,11 +34,11 @@
   private String ciphers = DistributionConfig.DEFAULT_SSL_CIPHERS;
   private boolean requireAuth = 
DistributionConfig.DEFAULT_SSL_REQUIRE_AUTHENTICATION;
   private String keystore = DistributionConfig.DEFAULT_SSL_KEYSTORE;
-  private String keystoreType = 
DistributionConfig.DEFAULT_CLUSTER_SSL_KEYSTORE_TYPE;
+  private String keystoreType = KeyStore.getDefaultType();
--- End diff --

I notice that `DistributionConfig.DEFAULT_CLUSTER_SSL_KEYSTORE_TYPE` is 
marked as deprecated. Is this its replacement?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---