Re: Review Request 53784: GEODE-1247: unable to stop server using http connection

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 15, 2016, 7:06 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53784/
> ---
> 
> (Updated Nov. 15, 2016, 7:06 a.m.)
> 
> 
> Review request for geode, Jens Deppe, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1247: unable to stop server using http connection
> 
> I believe after we added the "com.fasterxml.jackson.databind.ObjectMapper" 
> into core, the a mapper is added to the RestTemplate to precedes our own 
> message converter (SerializableObjectHttpMessageConverter), so on the client 
> side, we are using Jackson's converter to convert the message, but on the 
> server side, we are using our own converter, this caused the message corrupt 
> error. Fix is to remove the Jackson's converter from the client side.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java 
> 4c1cc5f79a3bed922347835441f6624931bdcc6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java
>  25eee8295ac553da416eb928b160fe7897b17763 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53784/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53784: GEODE-1247: unable to stop server using http connection

[Kevin Duling]

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



Why would we move away from using Jackson in favor of our own JSON mapping 
code?  Or is this a special case where we do some magic serialization we can't 
do via Jackson?

- Kevin Duling


On Nov. 15, 2016, 7:06 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53784/
> ---
> 
> (Updated Nov. 15, 2016, 7:06 a.m.)
> 
> 
> Review request for geode, Jens Deppe, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1247: unable to stop server using http connection
> 
> I believe after we added the "com.fasterxml.jackson.databind.ObjectMapper" 
> into core, the a mapper is added to the RestTemplate to precedes our own 
> message converter (SerializableObjectHttpMessageConverter), so on the client 
> side, we are using Jackson's converter to convert the message, but on the 
> server side, we are using our own converter, this caused the message corrupt 
> error. Fix is to remove the Jackson's converter from the client side.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/util/IOUtils.java 
> 4c1cc5f79a3bed922347835441f6624931bdcc6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/web/shell/AbstractHttpOperationInvoker.java
>  25eee8295ac553da416eb928b160fe7897b17763 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53784/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53685: GEODE-1955: properly disconnect gfsh session so that it won't leave heartbeat thread around to pollute other tests

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 14, 2016, 12:56 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53685/
> ---
> 
> (Updated Nov. 14, 2016, 12:56 p.m.)
> 
> 
> Review request for geode, Kevin Duling, Kirk Lund, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * add the "disconnect" command when we close the GfshShellConnectionRule so 
> that no heartbeat thread is left to pollute other tests.
> * Fix the test so that it truely test the jmx ssl connection, and fix the 
> configuration mishaps.
> * The above fix revealed another bug (GEODE-2099: race condition), ignored 
> the other two tests for now.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
>  76fb04169f06f82022858cbd0a03eadac8a42ef6 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java
>  0467c486a0e343bf745fe743172ed4fce750c5b4 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
>  456f76bb5592b000c75fe733553219c09d1b5381 
>   
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
>  b6afcca7cf8a27c2ad90d6ea570755a63ac0e89b 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
>  44da08be416240f49cbe9dfb8653f41eaea8777e 
> 
> Diff: https://reviews.apache.org/r/53685/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Where should geode example code live

[Kevin Duling]

+1

Shouldn't that be org.apache.geode.security.examples?

On Thu, Nov 10, 2016 at 11:03 AM, Jinmei Liao  wrote:

> +1 for both renaming it now and moving it to a separate module going
> forward.
>
> On Thu, Nov 10, 2016 at 10:58 AM, Kirk Lund  wrote:
>
> > We have some example code that's not in geode-examples.
> > geode-core-1.0.0-incubating.jar currently contains this package with
> > examples:
> >
> > org.geode.gemfire.security.templates
> >
> > Given that template means something a bit different from example, I'd
> like
> > to rename this package to examples. Is this acceptable?
> >
> > Going forward, this community needs to spend some considerable effort in
> > improving modularity of geode. The geode-core module should contain only
> > the bare essentials that are always necessary when using geode. Optional
> > features and the like should move to their own modules. Along the same
> line
> > of thought, examples should move to geode-examples or some similar
> > sub-module (or even a separate repo). That sub-module should generate a
> jar
> > that can be added to the classpath for purposes of demoing or trying out
> > geode (kicking the tires). I'd like the community to come up with
> > additional ideas on how to facilitate demoing and also make it easier in
> > general to try out geode for the first time.
> >
> > -Kirk
> >
>
>
>
> --
> Cheers
>
> Jinmei
>

Re: Review Request 53587: GEODE-2014: explicitly declare spring-web as a direct dependency rather than a transitive one.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 8, 2016, 2:31 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53587/
> ---
> 
> (Updated Nov. 8, 2016, 2:31 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2014: explicitly declare spring-web as a direct dependency rather than 
> a transitive one.
> 
> 
> Diffs
> -
> 
>   geode-web-api/build.gradle 9b1551633fc08296aab75dde4c251d086f583f82 
> 
> Diff: https://reviews.apache.org/r/53587/diff/
> 
> 
> Testing
> ---
> 
> dependency check
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53553: GEODE-2082: fix flakiness in sameKeepsOneFile

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 7, 2016, 11:25 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53553/
> ---
> 
> (Updated Nov. 7, 2016, 11:25 a.m.)
> 
> 
> Review request for geode, Darrel Schneider, Jinmei Liao, and Kevin Duling.
> 
> 
> Bugs: GEODE-2082
> https://issues.apache.org/jira/browse/GEODE-2082
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * rename sameKeepsOneFile to aboveZeroKeepsAtLeastOneFile
> * change test to be more flexible due to differences in file systems
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/statistics/DiskSpaceLimitIntegrationTest.java
>  2381a538e310467e97ae37003f04f22b149c367e 
> 
> Diff: https://reviews.apache.org/r/53553/diff/
> 
> 
> Testing
> ---
> 
> DiskSpaceLimitIntegrationTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 53579: GEODE-1570: add a test to verify rest security with SSL.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 8, 2016, 10:13 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53579/
> ---
> 
> (Updated Nov. 8, 2016, 10:13 a.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: add a test to verify rest security with SSL.
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
>  071b95ca8096f1e7872c9780fbf9c23bad92dc25 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java
>  PRE-CREATION 
>   geode-assembly/src/test/resources/ssl/trusted.keystore PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
>  72627a08ab5bbd6756c81b2bc332e97ac8692976 
> 
> Diff: https://reviews.apache.org/r/53579/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53577: Remove illegal javadoc tag that generates warning

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 8, 2016, 9:54 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53577/
> ---
> 
> (Updated Nov. 8, 2016, 9:54 a.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> @return and @param tags must have string value or it's illegal and generates 
> warning.
> 
> This change deletes an @return javadoc that has no value.
> 
> Here's the warning that the build reports:
> 
> :geode-core:javadoc
> /export/latvia1/users/klund/dev/gemfire/open/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java:447:
>  warning - @return tag has no arguments.
> 1 warning
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  7a898d13a387a8551ac6f6e5433891f6db3653d2 
> 
> Diff: https://reviews.apache.org/r/53577/diff/
> 
> 
> Testing
> ---
> 
> build javadoc
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 53543: GEODE-2079: mark the test as flaky

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Nov. 7, 2016, 8:42 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53543/
> ---
> 
> (Updated Nov. 7, 2016, 8:42 a.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2079: mark the test as flaky
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
>  1c8079aafa32b2848dfae573f2cd4e8e9f28cc5b 
> 
> Diff: https://reviews.apache.org/r/53543/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Gfsh Parsing

[Kevin Duling]

+1

On Fri, Nov 4, 2016 at 11:14 AM, Jens Deppe  wrote:

> One of the 'features' of the current output is for the text to be able to
> indicate whether the command is currently available or not. Personally I
> don't think there's much value to that. If necessary, more information
> could always be added to the help text.
>
> Definitely +1 for removing code complexity.
>
> --Jens
>
> On Fri, Nov 4, 2016 at 11:06 AM, Jared Stewart 
> wrote:
>
> > Huge +1 to the idea of simplifying Gfsh parsing.  I find the green help
> > text from Spring Shell too be less readable then the black text from
> > JoptSimple, but I assume we can configure that to our choosing.
> >
> > > On Nov 4, 2016, at 10:05 AM, Jinmei Liao  wrote:
> > >
> > > This is a good idea. I'll reach out to find it out. Thanks!
> > >
> > > On Fri, Nov 4, 2016 at 9:48 AM, Michael Stolz 
> wrote:
> > >
> > >> Can we suggest improvements to the Spring help capabilities? The
> Spring
> > >> community tends to be very responsive to good suggestions.
> > >>
> > >> --
> > >> Mike Stolz
> > >> Principal Engineer - Gemfire Product Manager
> > >> Mobile: 631-835-4771
> > >> On Nov 4, 2016 8:27 AM, "Jinmei Liao"  wrote:
> > >>
> > >>> We have several jira issues related to gfsh parsing (GEODE-1598,
> > >>> GEODE-1912). After spending some time understanding how the parsing
> > >> works,
> > >>> I found out we have three components intertwined together all trying
> to
> > >> do
> > >>> parsing: Gfsh, JoptSimple and Spring Shell. I started an experiment
> by
> > >>> getting rid of Gfsh and JoptSimple parsing and only using Spring
> Shell.
> > >> The
> > >>> outcome is I am able to maintain the current parsing and tabbing
> > >> completion
> > >>> capabilities (and fix a few bugs) by removing 40+ files. The only
> > >>> difference I see so far lies in the help and hint messages. It seems
> > the
> > >>> main reason we are using these home backed Gfsh parsing is to provide
> > >> more
> > >>> readable help messages. Below are the differences:
> > >>>
> > >>> Using Spring Shell's provided help:
> > >>>
> > >>> Using Gfsh Parsing with JoptSimple:
> > >>>
> > >>>
> > >>> ​
> > >>> ​I do like the outcome of the latter, but added complexity of the
> code
> > is
> > >>> too expensive to bear. So I am asking the community how important it
> is
> > >> to
> > >>> maintain the current style of help? Can we do with the cheaper way by
> > >> just
> > >>> using whatever provided by the libraries?
> > >>>
> > >>> --
> > >>> Cheers
> > >>>
> > >>> Jinmei
> > >>>
> > >>
> > >
> > >
> > >
> > > --
> > > Cheers
> > >
> > > Jinmei
> >
> >
>

Re: Review Request 53274: GEODE-1955: fix flaky tests by properly closing the gfsh instance in ConnectToLocatorSSLDUnitTest

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 28, 2016, 2:49 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53274/
> ---
> 
> (Updated Oct. 28, 2016, 2:49 p.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1955: fix flaky tests by properly closing the gfsh instance in 
> ConnectToLocatorSSLDUnitTest
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
>  8426c6f52129472c9c0def172aeefd10f1491935 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  d3390ba6ce2990d05516b701f82d5f176e6f4d49 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  b2dc6fe334499456184e1d0cbc0297063d4c99ce 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
>  25780e67dd3f7920e53162ef9400d2845e4c958b 
> 
> Diff: https://reviews.apache.org/r/53274/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53241: GEODE-1912: make ServerStarter and LocatorStarter as regular rules so that it's easier to use them in a RuleChain

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 27, 2016, 2:31 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53241/
> ---
> 
> (Updated Oct. 27, 2016, 2:31 p.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1912: make ServerStarter and LocatorStarter as regular rules so that 
> it's easier to use them in a RuleChain
> 
> * refactor gfshConectionRule as well
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  2dffcb713d761ace4b39aa9cf58fdea1d510af0b 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java
>  552a184bb9fd47dc70f1d0ab6022af18aaa7233e 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java
>  55cc26e28e44bc3be6beeb11e441b55046fcc3a8 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  bc25567e12e35e98d74edbd68acda7379503a895 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  992c27d5e2216879d66d9b8f1425ec2b5de0eeb1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  c9ae99700061865874fbcc691a0281f2cbf5075b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  60f58da8e1d047b9dde772d60eba4ada4428e9af 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  ce9c21c202d35202fb4b8bdf84fa90b100fa2c47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  e7724f75fd23f3530b6b2e3214b6b1bfb2d1cca9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  f754f2e04fe638d6c9b4cc9c31f513f4b47613ea 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  b9efe072d4074a5145efa74b5e69338e16f1d35c 
>   
> geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
>  cbe4a3bb4204fcf0e5482efb4e682bb9633eee04 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
>  84a4dd457274e3cd6cf1e632842ffa73846abad9 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
>  577a7a1d41476be87f35d5abbba68617d5e8db51 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
>  a4b25f52faa1c19168bef71f02897daa4675f39d 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
>  87314ed5b87b409f340ec75c3bcbd7fb838db57c 
>   
> geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
>  3cafdace9fe10a613a4b16989c87dfdf0bf49a99 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  ba78fdcf731ad30775357b90766d7757705c 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarter.java
>  216acef2036b7a53930a05acf95ef15bef6b5759 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarter.java 
> 22d3c5671a8faa9f3ee7e6f5051e38e0967c6972 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java
>  a4162fd9cf6c6e3aaf9215ae108fd9f6d0239180 
> 
> Diff: https://reviews.apache.org/r/53241/diff/
> 
> 
> Testing
> ---
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Code Formatting in develop

[Kevin Duling]

I just submitted a PR to geode that Travis CI rejected because of code
formatting in classes I had not modified.  Prior to my commit, I'd merged
in the latest develop branch this morning, which picked this up.

The error message reads:

> > Format violations were found. Run 'gradlew spotlessApply' to fix them.
>
> geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelQueueRemovalMessage.java
>
> geode-core/src/test/java/org/apache/geode/internal/cache/wan/parallel/ParallelQueueRemovalMessageJUnitTest.java
>
> geode-core/src/test/java/org/apache/geode/internal/cache/BucketRegionQueueHelper.java


My pull request https://github.com/apache/incubator-geode/pull/273 is held
up until this is corrected.

Re: Review Request 53198: GEODE-17: Fix a logical bug that prevents customers to set both Peer and Client authenticator

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 26, 2016, 10:42 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53198/
> ---
> 
> (Updated Oct. 26, 2016, 10:42 a.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-17: Fix a logical bug that prevents customers to set both peer and 
> Client authenticator
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  9f4697f6c9f06dbfd671e9e0d80dc52f447e5829 
>   
> geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
>  ee76dfc095c3aeb43a04cee899ba4b434c1e552e 
> 
> Diff: https://reviews.apache.org/r/53198/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: [GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

[Kevin Duling]

All of those files are the old Swagger-UI, replaced by the following gradle
entry in geode-web-api.  There is no reason we should be bundling the
swagger ui github project in to Geode.  I'm not even sure this old UI would
work with the Swagger2 API.  None of these are related to pulse, that lives
in geode-web, not geode-web-api.

compile('io.springfox:springfox-swagger-ui:' + project.'springfox.version') {
  exclude module: 'slf4j-api'
}


On Tue, Oct 25, 2016 at 11:09 AM, Anthony Baker  wrote:

> I missed these changes in the original pull request:
>
>  delete mode 100644 geode-web-api/src/main/webapp/docs/css/reset.css
>  delete mode 100644 geode-web-api/src/main/webapp/docs/css/screen.css
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/explorer_icons.png
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/logo_small.png
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/pet_store_api.png
>  delete mode 100755 geode-web-api/src/main/webapp/docs/images/throbber.gif
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/wordnik_api.png
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/backbone-min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/handlebars-1.0.0.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/highlight.7.3.pack.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery-1.8.0.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.ba-bbq.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.slideto.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.wiggle.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/shred.bundle.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/shred/content.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/swagger-oauth.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/swagger.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/underscore-min.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/o2c.html
>  delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.min.js
>
> These changes affect the contents of LICENSE (both ./LICENSE and
> geode-assembly/src/main/dist/LICENSE) as well as gradle/rat.gradle.
>
> Can you update the above files to reflect these bundling changes?  Some of
> the above libraries may also be in use by Pulse.
>
> Thanks,
> Anthony
>
>
>
>
> > On Oct 24, 2016, at 12:03 PM, asfgit  wrote:
> >
> > Github user asfgit closed the pull request at:
> >
> >https://github.com/apache/incubator-geode/pull/265
> >
> >
> > ---
> > 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 53171: GEODE-17: mark deprecated security configurations

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 25, 2016, 10:02 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53171/
> ---
> 
> (Updated Oct. 25, 2016, 10:02 a.m.)
> 
> 
> Review request for geode, Kevin Duling, Kirk Lund, and Swapnil Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-17: mark deprecated security configurations
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  38bec6f4d549bcaad9eb8b0dfe1bf9562919d72a 
> 
> Diff: https://reviews.apache.org/r/53171/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 21, 2016, 7:29 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53079/
> ---
> 
> (Updated Oct. 21, 2016, 7:29 a.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * add more test assertions.
> * fix legacy tests
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  6e91894c2c50e21c1fa250330119a7013f6b0fa5 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
>  30c8b3aa160648cae2ae2176e746ff987aba3aec 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
>  e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
>  ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
>  d13c99c11151355f3595718b15505db779d4161e 
> 
> Diff: https://reviews.apache.org/r/53079/diff/
> 
> 
> Testing
> ---
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Kevin Duling]

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



We've just stomped all over each other in RestSecurityIntegrationTest.  Some 
time back, you'd refactored RestSecurityEndpointsDUnitTest to change it from a 
DUnit to an Integration test.  But in that change, you removed the abstract 
base class I'd placed doGet, doPut, etc., in to.

With the test I need for the new Swagger, I re-introduced the base class to 
avoid a lot of code duplication.  As a result, your changes are incompatible 
with the changes I have in GEODE-2014.

The changes aren't large, but I wanted to point out the merge conflict we're 
going to have.

- Kevin Duling


On Oct. 20, 2016, 3:31 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53079/
> ---
> 
> (Updated Oct. 20, 2016, 3:31 p.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * add more test assertions.
> * fix legacy tests
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  6e91894c2c50e21c1fa250330119a7013f6b0fa5 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
>  30c8b3aa160648cae2ae2176e746ff987aba3aec 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
>  e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
>  ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
>  d13c99c11151355f3595718b15505db779d4161e 
> 
> Diff: https://reviews.apache.org/r/53079/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Updating Swagger

[Kevin Duling]

I've updated the page to appear as follows.  Any feedback is welcome.

https://drive.google.com/file/d/0B-lWOyam73gWb0hWT1Jtd1loeFE/view?usp=sharing

On Thu, Oct 20, 2016 at 11:40 AM, Kevin Duling <kdul...@pivotal.io> wrote:

> I'm updating Swagger to the Swagger2 specification and the new landing
> page has different fields than the old one.  I've populated most of them
> with some possible values and am looking for feedback.  I've placed a
> screenshot at https://drive.google.com/open?id=0B-
> lWOyam73gWdWZjVDloVF82VnM
>
> Some of the changes I think need to be made:
>
>- Change the "see more" to point to geode.apache.org
>- Change the "Created by" text to "the Apache Geode Community"
>- Change the email to dev@geode.incubator.apache.org
>- I need to know what version number to put in for the api.  I don't
>know if that's been defined, but I can't leave it as 'VERSION'
>
> Things I can't change easily:
>
>- "Created by _"  This header is generated by the Contact object
>sent in.
>- "See more at " Another auto-generated header
>- "Contact the developer" I can change what the link points to, but
>not the text
>- "API VERSION: " cannot be suppressed
>- Can't suppress the subject in the email link
>- almost everything in the gray text
>
>
>

Updating Swagger

[Kevin Duling]

I'm updating Swagger to the Swagger2 specification and the new landing page
has different fields than the old one.  I've populated most of them with
some possible values and am looking for feedback.  I've placed a screenshot
at https://drive.google.com/open?id=0B-lWOyam73gWdWZjVDloVF82VnM

Some of the changes I think need to be made:

   - Change the "see more" to point to geode.apache.org
   - Change the "Created by" text to "the Apache Geode Community"
   - Change the email to dev@geode.incubator.apache.org
   - I need to know what version number to put in for the api.  I don't
   know if that's been defined, but I can't leave it as 'VERSION'

Things I can't change easily:

   - "Created by _"  This header is generated by the Contact object
   sent in.
   - "See more at " Another auto-generated header
   - "Contact the developer" I can change what the link points to, but not
   the text
   - "API VERSION: " cannot be suppressed
   - Can't suppress the subject in the email link
   - almost everything in the gray text

Re: Review Request 52994: GEODE-1959: prompt for password when starting a server if username is specified

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 18, 2016, 1:28 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52994/
> ---
> 
> (Updated Oct. 18, 2016, 1:28 p.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1959: prompt for password when starting a server if username is 
> specified
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommandsDUnitTest.java
>  490e309dad7ee5100a2c39fc8ec3b13bc9c30737 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
>  4ffe08271d0e06919f45769f200b71ef0978d879 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  792a8abed97dcff8c15f678a4cd12e0d1656568d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  51887cf500f6aed00f79c10b22f708b03a13d178 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  467682d9b931e0bdd03c8dd3a739b686a6ace1c3 
>   
> geode-core/src/test/resources/org/apache/geode/management/internal/cli/commands/golden-help-offline.properties
>  28083f3058655b46c027c23dd64225f449ddb84e 
> 
> Diff: https://reviews.apache.org/r/52994/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52949: GEODE-2009: apply FlakyTest category to testCreateAlterDestroyUpdatesSharedConfig

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 17, 2016, 12:18 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52949/
> ---
> 
> (Updated Oct. 17, 2016, 12:18 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2009
> https://issues.apache.org/jira/browse/GEODE-2009
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2009: apply FlakyTest category to 
> testCreateAlterDestroyUpdatesSharedConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java
>  5fa06d9 
> 
> Diff: https://reviews.apache.org/r/52949/diff/
> 
> 
> Testing
> ---
> 
> build, CreateAlterDestroyRegionCommandsDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52937: GEODE-2007: fix unchecked warnings

[Kevin Duling]

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


Ship it!




Huzzah for reducing warnings!

- Kevin Duling


On Oct. 17, 2016, 12:52 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52937/
> ---
> 
> (Updated Oct. 17, 2016, 12:52 p.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2007
> https://issues.apache.org/jira/browse/GEODE-2007
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Fix the gradle build warnings for "uses unchecked or unsafe operations"
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-tomcat7/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession7.java
>  0df05ff 
>   
> extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/Tomcat8SessionsClientServerDUnitTest.java
>  8b29048 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  6c195e7 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java
>  027a1b5 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/DeltaPropagationWithCQDUnitTest.java
>  3990265 
>   
> geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
>  0b48e16 
> 
> Diff: https://reviews.apache.org/r/52937/diff/
> 
> 
> Testing
> ---
> 
> build
> modified tests
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52956: GEODE-2011: add FlakyTest category to testNonPersistentServerRestartAutoSerializer

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 17, 2016, 1:03 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52956/
> ---
> 
> (Updated Oct. 17, 2016, 1:03 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Hitesh Khamesra, and Udo 
> Kohlmeyer.
> 
> 
> Bugs: GEODE-2011
> https://issues.apache.org/jira/browse/GEODE-2011
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2011: add FlakyTest category to 
> testNonPersistentServerRestartAutoSerializer
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 1afb1ad 
> 
> Diff: https://reviews.apache.org/r/52956/diff/
> 
> 
> Testing
> ---
> 
> build
> PdxClientServerDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52936: GEODE-2006: add FlakyTest category to testSelectCommand

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 17, 2016, 10:59 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52936/
> ---
> 
> (Updated Oct. 17, 2016, 10:59 a.m.)
> 
> 
> Review request for geode, Jinmei Liao and Kevin Duling.
> 
> 
> Bugs: GEODE-2006
> https://issues.apache.org/jira/browse/GEODE-2006
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2006: add FlakyTest category to testSelectCommand
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java
>  5417ccb 
> 
> Diff: https://reviews.apache.org/r/52936/diff/
> 
> 
> Testing
> ---
> 
> build, GemfireDataCommandsDUnitTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52931: GEODE-1993: allow LocatorServerStartupRule to save server's ports as well.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 17, 2016, 8:10 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52931/
> ---
> 
> (Updated Oct. 17, 2016, 8:10 a.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * added more tetss
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
>  72dbd1aff32c718c1e6f72b940ade523c9cd8847 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java
>  bb147c7a24868adf0aaa1e860ca6114ffa032afc 
>   
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
>  5364c910a4c4e9d24de8be26b63a62428f48d0a7 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
>  d3ed823887c15ca06aba76e20a964c25e5dee8b4 
>   
> geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
>  f6928bfa0db529c38b521d4bc72d013272981e1a 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  71894c8a27a8768db2e7e99b1a2b981253e4ec67 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
>  bcc5ab3bca842d741bced5c59117472c77659ba6 
> 
> Diff: https://reviews.apache.org/r/52931/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 14, 2016, 11:41 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 11:41 a.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java
>  136319c06d9bd1e46b452219bf2894cf969fc1f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  1377fb643d035ad911ee5ceb80b87dfd72855428 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
>  4beff0bd55d69601266a0a856c8dcb80ffd4e1f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanServerConnectionRule.java
>  92430327e999af627e88956fc24aa1592e9a000d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java
>  873b6491f6703378d87383d96dc35e299b19a3fa 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
>  e5cbd15da9274d2a86be399f45c9239b2ae373be 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermi

Re: Starting the server, prompt for username/password

[Kevin Duling]

security-username and security-password appear to be the properties most
commonly set/checked.  I could base prompting off of their existence and
null value.

On Thu, Oct 13, 2016 at 2:55 PM, Kirk Lund <kl...@apache.org> wrote:

> I think we should avoid using an exception for normal flow control. I'd
> provide a link to Ward Cunningham's wiki but his wiki is currently down.
>
> There should be a way to determine if the user has credentials before
> trying to connect.
>
> -Kirk
>
>
> On Thu, Oct 13, 2016 at 2:50 PM, Kevin Duling <kdul...@pivotal.io> wrote:
>
> > I'm working on GEODE-1959
> > <https://issues.apache.org/jira/browse/GEODE-1959> and
> > the use case goes like this:
> >
> > Create your gemfire.properites with a security manager.  For example:
> >
> > > security-manager=org.apache.geode.security.templates.
> > SampleSecurityManage
> >
> > But do not provide a username and password within it.  Point to a
> > security.json file that contains users and roles.  My startup line within
> > gfsh goes like this:
> >
> > *gfsh>*start locator --name=loc --classpath=/Users/kduling/geode/run
> > > --properties-file=./gemfire.properties
> >
> >
> > Then start a server with security:
> >
> > *gfsh>*start server --name=secured --classpath=/Users/kduling/geode/run
> > > --properties-file=./gemfire.properties --locators=127.0.0.1[10334]
> >
> >
> > This produces a GemFireSecurityException because there are no login
> > credentials set.  The entire stacktrace goes:
> >
> > objc[2117]: Class JavaLaunchHelper is implemented in both
> > > /Library/Java/JavaVirtualMachines/jdk1.8.0_
> 92.jdk/Contents/Home/jre/bin/
> > java
> > > and
> > > /Library/Java/JavaVirtualMachines/jdk1.8.0_
> 92.jdk/Contents/Home/jre/lib/
> > libinstrument.dylib.
> > > One of the two will be used. Which one is undefined.
> > > Exception in thread "main"
> > > org.apache.geode.security.GemFireSecurityException: Failed to find
> > > credentials from [10.0.2.35(secured:2117):1025]
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.
> > membership.GMSJoinLeave.attemptToJoin(GMSJoinLeave.java:416)
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.
> > membership.GMSJoinLeave.join(GMSJoinLeave.java:314)
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.mgr.
> > GMSMembershipManager.join(GMSMembershipManager.java:662)
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.mgr.
> > GMSMembershipManager.joinDistributedSystem(
> GMSMembershipManager.java:749)
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.
> > Services.start(Services.java:183)
> > > at
> > > org.apache.geode.distributed.internal.membership.gms.GMSMemberFactory.
> > newMembershipManager(GMSMemberFactory.java:104)
> > > at
> > > org.apache.geode.distributed.internal.membership.MemberFactory.
> > newMembershipManager(MemberFactory.java:92)
> > > at
> > > org.apache.geode.distributed.internal.DistributionManager.<
> > init>(DistributionManager.java:1092)
> > > at
> > > org.apache.geode.distributed.internal.DistributionManager.<
> > init>(DistributionManager.java:1144)
> > > at
> > > org.apache.geode.distributed.internal.DistributionManager.
> > create(DistributionManager.java:521)
> > > at
> > > org.apache.geode.distributed.internal.InternalDistributedSystem.
> > initialize(InternalDistributedSystem.java:657)
> > > at
> > > org.apache.geode.distributed.internal.InternalDistributedSystem.
> > newInstance(InternalDistributedSystem.java:297)
> > > at
> > > org.apache.geode.distributed.DistributedSystem.connect(
> > DistributedSystem.java:237)
> > > at org.apache.geode.cache.CacheFactory.create(
> CacheFactory.java:229)
> > > at
> > > org.apache.geode.distributed.internal.DefaultServerLauncherCacheProv
> > ider.createCache(DefaultServerLauncherCacheProvider.java:55)
> > > at
> > > org.apache.geode.distributed.ServerLauncher.createCache(
> > ServerLauncher.java:783)
> > > at
> > > org.apache.geode.distributed.ServerLauncher.start(
> > ServerLauncher.java:703)
> > > at
> > > org.apache.geode.distributed.ServerLauncher.run(
> ServerLauncher.java:633)
> > > at
> > > org.apache.geode.distributed.ServerLauncher.main(
> > ServerLauncher.java:184)
> >
> >
> > To resolve this, I was planning on catching the exception at
> > DistributedSystem.connect() and then prompting for the username/password
> > similar to what is done in ShellCommands.jmxConnect().
> >
> > Does anyone see a problem with this or have a recommendation for a better
> > approach?
> >
>

Starting the server, prompt for username/password

[Kevin Duling]

I'm working on GEODE-1959
 and
the use case goes like this:

Create your gemfire.properites with a security manager.  For example:

> security-manager=org.apache.geode.security.templates.SampleSecurityManage

But do not provide a username and password within it.  Point to a
security.json file that contains users and roles.  My startup line within
gfsh goes like this:

*gfsh>*start locator --name=loc --classpath=/Users/kduling/geode/run
> --properties-file=./gemfire.properties


Then start a server with security:

*gfsh>*start server --name=secured --classpath=/Users/kduling/geode/run
> --properties-file=./gemfire.properties --locators=127.0.0.1[10334]


This produces a GemFireSecurityException because there are no login
credentials set.  The entire stacktrace goes:

objc[2117]: Class JavaLaunchHelper is implemented in both
> /Library/Java/JavaVirtualMachines/jdk1.8.0_92.jdk/Contents/Home/jre/bin/java
> and
> /Library/Java/JavaVirtualMachines/jdk1.8.0_92.jdk/Contents/Home/jre/lib/libinstrument.dylib.
> One of the two will be used. Which one is undefined.
> Exception in thread "main"
> org.apache.geode.security.GemFireSecurityException: Failed to find
> credentials from [10.0.2.35(secured:2117):1025]
> at
> org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave.attemptToJoin(GMSJoinLeave.java:416)
> at
> org.apache.geode.distributed.internal.membership.gms.membership.GMSJoinLeave.join(GMSJoinLeave.java:314)
> at
> org.apache.geode.distributed.internal.membership.gms.mgr.GMSMembershipManager.join(GMSMembershipManager.java:662)
> at
> org.apache.geode.distributed.internal.membership.gms.mgr.GMSMembershipManager.joinDistributedSystem(GMSMembershipManager.java:749)
> at
> org.apache.geode.distributed.internal.membership.gms.Services.start(Services.java:183)
> at
> org.apache.geode.distributed.internal.membership.gms.GMSMemberFactory.newMembershipManager(GMSMemberFactory.java:104)
> at
> org.apache.geode.distributed.internal.membership.MemberFactory.newMembershipManager(MemberFactory.java:92)
> at
> org.apache.geode.distributed.internal.DistributionManager.(DistributionManager.java:1092)
> at
> org.apache.geode.distributed.internal.DistributionManager.(DistributionManager.java:1144)
> at
> org.apache.geode.distributed.internal.DistributionManager.create(DistributionManager.java:521)
> at
> org.apache.geode.distributed.internal.InternalDistributedSystem.initialize(InternalDistributedSystem.java:657)
> at
> org.apache.geode.distributed.internal.InternalDistributedSystem.newInstance(InternalDistributedSystem.java:297)
> at
> org.apache.geode.distributed.DistributedSystem.connect(DistributedSystem.java:237)
> at org.apache.geode.cache.CacheFactory.create(CacheFactory.java:229)
> at
> org.apache.geode.distributed.internal.DefaultServerLauncherCacheProvider.createCache(DefaultServerLauncherCacheProvider.java:55)
> at
> org.apache.geode.distributed.ServerLauncher.createCache(ServerLauncher.java:783)
> at
> org.apache.geode.distributed.ServerLauncher.start(ServerLauncher.java:703)
> at
> org.apache.geode.distributed.ServerLauncher.run(ServerLauncher.java:633)
> at
> org.apache.geode.distributed.ServerLauncher.main(ServerLauncher.java:184)


To resolve this, I was planning on catching the exception at
DistributedSystem.connect() and then prompting for the username/password
similar to what is done in ShellCommands.jmxConnect().

Does anyone see a problem with this or have a recommendation for a better
approach?

Re: Coding practices/standards

[Kevin Duling]

Given that, +1 from me!

On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart <jstew...@pivotal.io> wrote:

> The task is fully suppressible with -x spotlessCheck.  Also, if you have
> any formatter errors you can automatically fix them with 'gradle
> spotlessApply’.
>
> > On Oct 13, 2016, at 9:40 AM, Kevin Duling <kdul...@pivotal.io> wrote:
> >
> > If we made formatting a warning, then people would probably quickly
> ignore
> > it.
> > If we made formatting an error, we need to be sure we don't get in to the
> > situation where 's formatter is not in agreement with
> the
> > build's checker.
> >
> > I can live with an additional 17 seconds as well.  And Jared's already
> > reduced the build time locally by 50%.  But I still want the ability to
> > suppress the check similar to -x javadoc.
> >
> > On Wed, Oct 12, 2016 at 9:58 PM, William Markito <wmark...@pivotal.io>
> > wrote:
> >
> >> This sounds really good to me as well.  +1
> >>
> >> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart <jstew...@pivotal.io>
> >> wrote:
> >>
> >>> This is running locally on my laptop.  Since Spotless is only doing
> code
> >>> formatting and not any other static analysis, it already has 0 errors.
> >>> (Other than, of course, formatting not consistent with the template.)
> >>>
> >>>> On Oct 12, 2016, at 4:11 PM, Kenneth Howe <kh...@pivotal.io> wrote:
> >>>>
> >>>> Agree with Mark, this has to work with 0 errors before it would be
> >>> useful in precheckin. I think I could live with an additional 17
> seconds
> >>> most of the time for running the spotlessCheck as suggested.
> >>>>
> >>>> Jared, Is that 17 seconds running locally on your laptop or on a more
> >>> capable machine?
> >>>>
> >>>> Ken
> >>>>
> >>>>> On Oct 12, 2016, at 3:39 PM, Jared Stewart <jstew...@pivotal.io>
> >> wrote:
> >>>>>
> >>>>> If you want to try it out, I pushed a branch to my Geode repo that
> >>> contains this change:
> >>>>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> >> <
> >>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> >>>>>
> >>>>>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider <
> dschnei...@pivotal.io
> >>>
> >>> wrote:
> >>>>>>
> >>>>>> I like Dan's idea of catching formatting issues earlier but I think
> >>> adding
> >>>>>> 5-10 minutes to "build" would be too much. Currently when I'm trying
> >>> to do
> >>>>>> a quick build I use -xjavadoc. I'd probably do the same for this
> >>> target if
> >>>>>> it was part of build until I'm ready to do a precheckin.
> >>>>>>
> >>>>>> Mark, wouldn't running the formatter on all our java files and
> >> checking
> >>>>>> them in get these issues down to 0?
> >>>>>>
> >>>>>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> >> ukohlme...@pivotal.io
> >>>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> +1 - adding checkstyle to precheckin.
> >>>>>>>
> >>>>>>> If the developer uses the provided templates ( eclipse + intellij)
> >>> then
> >>>>>>> most of the formatting issues should be handled before precheckin.
> >>> Also, if
> >>>>>>> a developer has a questionable coding style, that should lessen as
> >>> that
> >>>>>>> developer will have resolve the issues before being able to commit.
> >>>>>>>
> >>>>>>> I also believe that this should not be an overbearing and intrusive
> >>>>>>> process.
> >>>>>>>
> >>>>>>> --Udo
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 13/10/16 6:36 am, Mark Bretl wrote:
> >>>>>>>
> >>>>>>>> Dan,
> >>>>>>>>
> >>>>>>>> There is some extra amount of time, 5-10 minutes extra for the
> >> entire
> >>>>>>>> pr

Re: Coding practices/standards

[Kevin Duling]

If we made formatting a warning, then people would probably quickly ignore
it.
If we made formatting an error, we need to be sure we don't get in to the
situation where 's formatter is not in agreement with the
build's checker.

I can live with an additional 17 seconds as well.  And Jared's already
reduced the build time locally by 50%.  But I still want the ability to
suppress the check similar to -x javadoc.

On Wed, Oct 12, 2016 at 9:58 PM, William Markito 
wrote:

> This sounds really good to me as well.  +1
>
> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 
> wrote:
>
> > This is running locally on my laptop.  Since Spotless is only doing code
> > formatting and not any other static analysis, it already has 0 errors.
> > (Other than, of course, formatting not consistent with the template.)
> >
> > > On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> > >
> > > Agree with Mark, this has to work with 0 errors before it would be
> > useful in precheckin. I think I could live with an additional 17 seconds
> > most of the time for running the spotlessCheck as suggested.
> > >
> > > Jared, Is that 17 seconds running locally on your laptop or on a more
> > capable machine?
> > >
> > > Ken
> > >
> > >> On Oct 12, 2016, at 3:39 PM, Jared Stewart 
> wrote:
> > >>
> > >> If you want to try it out, I pushed a branch to my Geode repo that
> > contains this change:
> > >> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> <
> > https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> > >>
> > >>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  >
> > wrote:
> > >>>
> > >>> I like Dan's idea of catching formatting issues earlier but I think
> > adding
> > >>> 5-10 minutes to "build" would be too much. Currently when I'm trying
> > to do
> > >>> a quick build I use -xjavadoc. I'd probably do the same for this
> > target if
> > >>> it was part of build until I'm ready to do a precheckin.
> > >>>
> > >>> Mark, wouldn't running the formatter on all our java files and
> checking
> > >>> them in get these issues down to 0?
> > >>>
> > >>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> ukohlme...@pivotal.io
> > >
> > >>> wrote:
> > >>>
> >  +1 - adding checkstyle to precheckin.
> > 
> >  If the developer uses the provided templates ( eclipse + intellij)
> > then
> >  most of the formatting issues should be handled before precheckin.
> > Also, if
> >  a developer has a questionable coding style, that should lessen as
> > that
> >  developer will have resolve the issues before being able to commit.
> > 
> >  I also believe that this should not be an overbearing and intrusive
> >  process.
> > 
> >  --Udo
> > 
> > 
> > 
> >  On 13/10/16 6:36 am, Mark Bretl wrote:
> > 
> > > Dan,
> > >
> > > There is some extra amount of time, 5-10 minutes extra for the
> entire
> > > project (depending on your CPU). I think the real issue to adding
> it
> > to
> > > the
> > > precheckin target and have it be 'effective' is it needs to run
> > > successfully, otherwise it would turn into noise most of the time I
> > think.
> > > We need to get the issues down to 0 or manage to set a new baseline
> > (not
> > > the best idea), which is a lot of work, to make it run
> successfully.
> > Right
> > > now, if you run the target, it will fail every time since there
> > > outstanding
> > > issues in the code and very hard to tell what issues were
> introduced.
> > >
> > >
> > > --Mark
> > >
> > > On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
> > wrote:
> > >
> > > Seems like it should run as part of the build target. The only
> > reason to
> > >> make it part of precheckin is if it takes a long time, otherwise
> > people
> > >> should get fast feedback they need to change their code before
> they
> > push.
> > >>
> > >> -Dan
> > >>
> > >> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <
> > jstew...@pivotal.io>
> > >> wrote:
> > >>
> > >> +1 to running during the precheckin target as well as Travis CI
> > >>>
> > >>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
> > dschnei...@pivotal.io>
> > >>> wrote:
> > >>>
> > >>> If Travis CI is only run on pull requests then that is not enough
> > 
> > >>> because
> > >>
> > >>> committers do not submit pull requests. Having it run during the
> > gradle
> >  build or precheckin target is also needed. In addition to that I
> > also
> >  wanted PRs to be checked.
> > 
> > 
> >  On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <
> > jstew...@pivotal.io>
> >  wrote:
> > 
> >  It would certainly be necessary to make sure that the code style
> > to
> > >
> >  be
> > 

Re: Coding practices/standards

[Kevin Duling]

I know I've been guilty of submitting code for review that hasn't matched
the code style in the past.  As a result, I've checked the "reformat code"
box on the 'commit' dialog in IDEA, pointing to the IntelliJ xml formatter.

It's frustrating to me that this sort of thing is still an issue in
software development.  Ever since I've started my career, there's been
arguments about whitespace; how far to indent and what line the curly brace
goes on.  And I'm yet to be convinced one way is superior to the other.
What is easier for one to read is more difficult for another.

While I'm developing and doing frequent local builds, I do not want a build
to fail due to whitespace issues.

So I am in favor of adding this if there is an option to disable like we
can disable javadoc and tests, but enforce it by default and/or precheckin.


On Wed, Oct 12, 2016 at 10:07 AM, Nabarun Nag  wrote:

> +1
>
> On Wed, Oct 12, 2016 at 10:06 AM Dan Smith  wrote:
>
> > +1
> >
> > This might be a good time to reformat the code since I don't think there
> > are too many long lived feature branches outstanding.
> >
> > -Dan
> >
> > On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> > wrote:
> >
> > > I would like to advocate for adding a Checkstyle  > > sourceforge.net/> or Spotless 
> > > gradle task to our build process to ensure that all code checked in
> meets
> > > the formatting standards described on the wiki <
> > https://cwiki.apache.org/
> > > confluence/display/GEODE/Code+Style+Guide> (and in the
> intellij/eclipse
> > > formatter xml files in our repository). This will alleviate
> difficulties
> > > reviewing code when whitespace or formatting has changed since all code
> > > checked in will already comply with standards.
> >
>

Re: Review Request 52751: GEODE-1986: correctly set the flag indicating if cluster configuration service is running or not on a locator.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 11, 2016, 7:21 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52751/
> ---
> 
> (Updated Oct. 11, 2016, 7:21 p.m.)
> 
> 
> Review request for geode, Jared Stewart, John Blum, Kevin Duling, and Kirk 
> Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1986: correctly set the flag indicating if cluster configuration 
> service is running or not on a locator.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
>  17303c1f984a90159f924a262d0ec843410cec16 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  d166397e3d5897347500e5447637fb4b99895388 
>   
> geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithEmbededLocatorDUnitTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52751/diff/
> 
> 
> Testing
> ---
> 
> prechecking running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52742: GEODE-1979: refactor tests using LocatorServerConfigurationRule

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 11, 2016, 6:11 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52742/
> ---
> 
> (Updated Oct. 11, 2016, 6:11 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1979: refactor tests using LocatorServerConfigurationRule
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
>  ff7f0282ae0bd0bed74aca6e549c8b3162d76900 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
>  877f0571d6b6985d0cda43c35f0abae6cbc12a88 
>   
> geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
>  dae062acb980a93d00b5498e6ac99eda97918a39 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
>  2fa262c732662becbba0eb48ce47e7d88b0fec5c 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
>  f837530e2bcd6026102d5a1267a48ac8b37578fe 
> 
> Diff: https://reviews.apache.org/r/52742/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Kevin Duling]

> On Oct. 5, 2016, 2:18 p.m., Jared Stewart wrote:
> > Aren't the (OutputStream) casts in JSONUtils.java redundant since 
> > outputStream here is already an instance of HeapDataOutputStream which 
> > extends OutputStream?
> > 
> > Also, I believe our schemaLocations in *-servlet.xml ought to refer to 
> > versioned URLs so that we do not break when a new Spring version is 
> > released before we have updated to that version.

When I was trying to do this ticket, I ran in to the necessity of having to 
cast because the compiler objected, saying there was no matching method that 
would accept it -- even though it clearly appeared there was.  By casting, it 
resolved it.  I'm sure Jinmei ran in to the same issue when she worked on the 
upgrade.


- Kevin


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


On Oct. 5, 2016, 2:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 2:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 5, 2016, 2:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 2:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52525: GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE

[Kevin Duling]

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


Ship it!




Some things to consider:
* * Your teardown code is identical to the teardown code in 
CustomConfigWithLogServiceIntegrationTest.
* Your preassertions are repeated in createConfigFile.
* You've tested for ACCEPT and DENY.  Is there value in testing for NEUTRAL?
* Are you sure you want to be using @Before and not @BeforeClass?  @Before is 
called before each and every test.  @BeforeClass is only called once.  This is 
causing you to call preassert() eight times.  Then createConfigFile() is called 
a total of thirty-two times.

- Kevin Duling


On Oct. 4, 2016, 1:23 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52525/
> ---
> 
> (Updated Oct. 4, 2016, 1:23 p.m.)
> 
> 
> Review request for geode and Kevin Duling.
> 
> 
> Bugs: GEODE-1902
> https://issues.apache.org/jira/browse/GEODE-1902
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1902: add ACCEPT and DENY tests for GEODE_VERBOSE and GEMFIRE_VERBOSE
> 
> log4j2-custom.xml isn't really sufficient for testing GEODE_VERBOSE and 
> GEMFIRE_VERBOSE. This changeset adds in new 4 log4j2 xml files that enable 
> and disable each of the VERBOSE markers.
> 
> * add two log4j2 xml files for GEMFIRE_VERBOSE: 
> log4j2-gemfire_verbose-accept.xml, log4j2-gemfire_verbose-deny.xml
> * add two log4j2 xml files for GEODE_VERBOSE: 
> log4j2-geode_verbose-accept.xml, log4j2-geode_verbose-deny.xml
> * rename test GeodeVerboseLogMarkerIntegrationTest
> * add test methods to verify every combination of behavior for accept and 
> deny xml files
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/Configuration.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/GeodeVerboseLogMarkerIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/logging/log4j/LogMarkerJUnitTest.java
>  ec9d257892b64bb6ad2fc58755a448c70ddf538d 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-accept.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-gemfire_verbose-deny.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-accept.xml
>  PRE-CREATION 
>   
> geode-core/src/test/resources/org/apache/geode/internal/logging/log4j/marker/log4j2-geode_verbose-deny.xml
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52525/diff/
> 
> 
> Testing
> ---
> 
> GeodeVerboseLogMarkerIntegrationTest
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 52468: Add FlakyTest category to tests with open bugs

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Oct. 1, 2016, 5:07 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52468/
> ---
> 
> (Updated Oct. 1, 2016, 5:07 p.m.)
> 
> 
> Review request for geode.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Annotate test methods with FlakyTest category if there is an open
> bug for that test. This will improve the signal/noise ratio for
> unit/integration/distributed tests.  Flaky tests are still run
> as part of precheckin and flakyTest targets.
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-session/src/test/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationEndToEndDUnitTest.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/controllers/RestAPIsOnGroupsFunctionExecutionDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/ConnectionPoolDUnitTest.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/management/MemoryThresholdsOffHeapDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/dunit/CompiledInDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/dunit/QueryUsingFunctionContextDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/internal/index/ConcurrentIndexOperationsOnOverflowRegionDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRQueryCacheCloseDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRQueryRegionCloseDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/DistributedSystemDUnitTest.java
>  PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/distributed/LocatorDUnitTest.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java 
> PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/SystemAdminDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/ConsoleDistributionManagerDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/distributed/internal/membership/MembershipJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/FixedPRSinglehopDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/FunctionServiceBase.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRClientServerRegionFunctionExecutionDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRClientServerRegionFunctionExecutionFailoverDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRClientServerRegionFunctionExecutionSelectorNoSingleHopDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRColocationDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/fixed/FixedPartitioningDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/Bug36805DUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/DurableRegistrationDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/w

Re: Review Request 52269: GEODE-1659: put security properties in the cluster config and applied to all the members in the cluster.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Sept. 26, 2016, 1:32 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52269/
> ---
> 
> (Updated Sept. 26, 2016, 1:32 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1659: put security properties in the cluster config and applied to all 
> the members in the cluster.
> 
> When joining a secured server, the server's security-manager and 
> security-post-processor should match whatever the cluster config is. 
> Also prevent a member not using cluster configuration from joining a secured 
> cluster.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java
>  2e6677f3dff5bc9a811185e9844b22e4ed925971 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
>  93a9c3b1686a82d8c7814fbda47650d4a26ebbd2 
>   
> geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
> d341f51f493dc4e0391be26b15d64d7568c35fcb 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
>  c42d1a259e21a02a4aee0dc3fd4c1a51779b79e3 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
>  597a977ccaab900fe38a4042b996daedaef99456 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
>  8468664446a3953bf5dfb056621bfcbdd0c8daa4 
> 
> Diff: https://reviews.apache.org/r/52269/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Brew formulae for geode

[Kevin Duling]

I'll be updating MacPorts probably this week, too, also calling it
apache-geode.

On Wed, Aug 24, 2016 at 7:43 AM, Anthony Baker  wrote:

> Bumping this old thread:
>
> I’ve updated the brew formula with the 1.0.0-incubating.M3 release and
> renamed it to apache-geode.
>
> `brew install apache-geode`
>
> Anthony
>
>
> > On Apr 27, 2016, at 6:51 AM, Anthony Baker  wrote:
> >
> > There’s a clarification thread here [1].  I’ll followup with the
> HomeBrew peoples and update this thread with the resolution.
> >
> > Anthony
> >
> > [1] http://mail-archives.apache.org/mod_mbox/incubator-
> general/201604.mbox/%3cCAEwge-GBVo5fVW7bXniwUXS-=F=
> lqfvhcskaur51cxo4ysv...@mail.gmail.com%3e
> >
> >> On Apr 23, 2016, at 10:36 PM, Roman Shaposhnik 
> wrote:
> >>
> >> On Sat, Apr 23, 2016 at 10:27 PM, Anthony Baker 
> wrote:
> >>> I can go either way, but the brew maintainers seem to prefer without
> the apache- prefix:
> >>> https://github.com/Homebrew/brew/blob/master/share/doc/
> homebrew/Formula-Cookbook.md
> >>>
> >>> Out of 57 Apache projects on Homebrew, only 6 use the apache- prefix:
> >>>
> >>> Anthony-Baker-MacBook-Pro:Formula abaker$ ls -1 apache* | wc -l
> >>>  6
> >>
> >> Not sure where how you're getting your # but
> >>  $ brew search apache
> >> apache-activemqapache-forrest
> >> apache-archiva apache-opennlp
> >> apache-drill   apache-spark
> >> apache-fop apachetop
> >>
> >>> Anthony-Baker-MacBook-Pro:Formula abaker$ grep -i apache * | grep
> homepage | wc -l
> >>> 57
> >>
> >> Well, personally I see that the rest of the projects are either way more
> >> unique in their naming than geode or are older generation (the time
> >> when branding requirements were less prominent).
> >>
> >> But I'm with you -- I was on the fence when I was writing the email
> myself.
> >> What convinced me to ask you to name it apache-geode was seeing
> >> apache-spark and apache-drill.
> >>
> >> Thanks,
> >> Roman.
> >
>
>

Re: git commit messages

[Kevin Duling]

The format is very similar to the one most other git shops I've worked in
before use.  I don't believe we ever had formal length limits.  Typically,
it was:

-: 
>
blank line




The Atlassian plugin for IDEA automates a lot of this.  There are limits on
the length of a jira ticket summary, but I'm not sure what that is.  I ran
in to it when I did my round of CI.

I don't see a reason to change anything except maybe stress that he lengths
are a guideline, not a hard & fast rule.  If more room is needed to write
good information, it shouldn't be truncated as it's not unknown to move
away from a given ticket system.

On Wed, Aug 17, 2016 at 3:38 PM, Kirk Lund  wrote:

> 50 chars including "GEODE-: " is awfully short.
> http://chris.beams.io/posts/git-commit/ does say that's just a general
> rule
> of thumb and not a hard limit. The author's reasoning seems to be
> specifically for using "git log --oneline" -- does anyone use that option
> with git log? I don't.
>
> I guess another option is to not have to have a guideline if we don't want
> one... our current git log messages are reasonable and make sense.
>
> -Kirk
>
>
> On Wed, Aug 17, 2016 at 3:21 PM, Kirk Lund  wrote:
>
> > Here's the git commit message guidelines we discussed and voted on last
> > year. I just checked and my own git commit message line lengths have
> grown
> > beyond what we decided to use. Most other are also not following this
> > guideline.
> >
> > Here's the list of folks who voted last year along with their vote:
> >
> > Anthony Baker +1
> > Vincent Ford +1
> > William Markito +1
> > arghya sadhu +1
> >
> > Do we want to reaffirm this guideline or should it change?
> >
> > -Kirk
> >
> > -- Forwarded message --
> > From: Kirk Lund 
> > Date: Wed, Aug 5, 2015 at 3:18 PM
> > Subject: git commit messages
> > To: dev@geode.incubator.apache.org
> >
> >
> > Several of us were discussing http://chris.beams.io/posts/git-commit/ --
> > there are a couple other really good articles about git commit messages
> and
> > below is the message style I've been trying to follow.
> >
> > http://chris.beams.io/posts/git-commit/
> > http://www.laurencegellert.com/2013/07/how-to-write-a-proper-commit-
> > message/
> > http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
> >
> > GEODE-nn: Begin capitalized and 50 chars or less
> >
> > More detailed explanation with linefeeds to wrap at 72 characters after
> > a blank line following the summary.
> >
> > Further paragraphs come after blank lines.
> >
> > - Bullet points are okay, too
> >
> > - Typically a hyphen or asterisk is used for the bullet, followed by a
> >   single space, with blank lines in between, but conventions vary here
> >
> > - Use a hanging indent
> >
> >
> >
> >
>

Re: Review Request 51098: GEODE-1781: repackage internal statistics classes and refactor statistics tests

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On Aug. 15, 2016, 6:39 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51098/
> ---
> 
> (Updated Aug. 15, 2016, 6:39 p.m.)
> 
> 
> Review request for geode, Darrel Schneider, Kevin Duling, Lynn 
> Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
> 
> 
> Bugs: GEODE-1781
> https://issues.apache.org/jira/browse/GEODE-1781
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1781: repackage internal statistics classes and refactor statistics 
> tests
> 
> * move internal statistics classes and tests into 
> com.gemstone.gemfire.internal.statistics pkg
> * modify tests to include integration and distributed in names
> * modify tests to use TemporaryFolder and TestName rules
> * remove unused classes and dead code from statistics tests
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/session/catalina/internal/DeltaSessionStatistics.java
>  2d59103 
>   
> extensions/geode-modules/src/main/java/com/gemstone/gemfire/modules/util/ModuleStatistics.java
>  82e882c 
>   geode-core/src/main/java/com/gemstone/gemfire/StatisticsFactory.java 
> 3ef5ba5 
>   geode-core/src/main/java/com/gemstone/gemfire/StatisticsTypeFactory.java 
> 9b6c546 
>   
> geode-core/src/main/java/com/gemstone/gemfire/admin/internal/MemberHealthEvaluator.java
>  d0710ca 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/asyncqueue/internal/AsyncEventQueueStats.java
>  8d541a3 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/ConnectionStats.java
>  4bd4439 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/EndpointManagerImpl.java
>  d155602 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/client/internal/PoolImpl.java
>  b26d76b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/CqQueryVsdStats.java
>  e9d30d5 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/IndexStats.java
>  77941b6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionStats.java
>  c4803ab 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java
>  1ea5611 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/LocatorStats.java
>  f59ed84 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/locks/DLockStats.java
>  9a61c10 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/AbstractStatisticsFactory.java
>  972e670e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/ArchiveSplitter.java 
> 28b9dd7 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/DummyStatisticsFactory.java
>  9d280eb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/DummyStatisticsImpl.java
>  aa8da50 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/GemFireStatSampler.java
>  1eb35d0 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatHelper.java 
> 7ff0b1b 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/HostStatSampler.java 
> ff9d2fb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LinuxProcFsStatistics.java
>  3070287 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LinuxProcessStats.java 
> 05bc284 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LinuxSystemStats.java 
> 22158da 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LocalStatListener.java 
> c46bc2f 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LocalStatisticsFactory.java
>  6f8315a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/LocalStatisticsImpl.java
>  cf59e1f 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/OSXProcessStats.java 
> 0706c7b 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/OSXSystemStats.java 
> 8d440ca 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/OsStatisticsFactory.java
>  7dac90e 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/ProcessStats.java 
> c80e340 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/SimpleStatSampler.java 
> b3cc56

Re: Review Request 51125: GEODE-1782: stat resources with different time stamps should not be equal

[Kevin Duling]

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



In the equals() method, there are several more fields that aren't compared.  
Should they be?

- Kevin Duling


On Aug. 15, 2016, 6:56 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51125/
> ---
> 
> (Updated Aug. 15, 2016, 6:56 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Darrel Schneider, Kevin Duling, Lynn 
> Hughes-Godfrey, Lynn Gallinat, and Dan Smith.
> 
> 
> Bugs: GEODE-1782
> https://issues.apache.org/jira/browse/GEODE-1782
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1782: stat resources with different time stamps should not be equal
> 
> * StatArchiveWithConsecutiveResourceInstGenerator generates gfs with multiple 
> stat resources of same name but different times
> * StatArchiveWithConsecutiveResourceInstIntegrationTest confirms existence of 
> bug GEODE-1782: StatArchiveReader ignores later stats reso
> urce with same name as closed stats resource
> * ResourceInstTest verifies the underlying issue in 
> StatArchiveReader.ResourceInst.equals and the fix
> 
> Note: this changeset builds upon the changeset for GEODE-1781 (same branch): 
> https://reviews.apache.org/r/51098
> 
> Explanation of new tests: I started out with a very large .gfs file and 
> reproduced the bug with 
> StatArchiveWithConsecutiveResourceInstIntegrationTest. Darrel helped identify 
> the underlying cause in StatArchiveReader.ResourceInst.equals. I then wrote 
> the unit test ResourceInstTest to reproduce the bug at the lowest level 
> possible and fix it. Next I created 
> StatArchiveWithConsecutiveResourceInstGenerator to generate a tiny .gfs file 
> for StatArchiveWithConsecutiveResourceInstIntegrationTest so that I could 
> check it in as well since it verifies the bug and the fix at a higher level.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/statistics/StatArchiveReader.java
>  12637bc 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/ResourceInstTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGenerator.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstGeneratorTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/statistics/StatUtils.java
>  PRE-CREATION 
>   
> geode-core/src/test/resources/com/gemstone/gemfire/internal/statistics/StatArchiveWithConsecutiveResourceInstIntegrationTest.gfs
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51125/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Trying to upgrade spring-security

[Kevin Duling]

Yes, that's my experience so far. But this particular error persists
regardless of how deep down the rabbit hole I gone and I haven't been able
to solve it.  I think it's due to an older slf4j being bundled in to jetty.

I'll give those commands a try and see whether they help or not.

On Tue, Aug 16, 2016 at 8:53 AM, Anthony Baker <aba...@pivotal.io> wrote:

> Not sure if this is related to your error, but it looks like that version
> of spring-security would at least require moving to spring framework 4.3.1
> and possibly more dep updates as well.  You should be able to run some
> gradle commands (dependencies, dependencyInsight, dumpDependencies) to help
> sort out the full list of version changes needed.
>
> Anthony
>
> > On Aug 16, 2016, at 8:39 AM, Kevin Duling <kdul...@pivotal.io> wrote:
> >
> > I'm working on adding security to the REST methods and wanted to start by
> > upgrading spring-security to the latest version.
> >
> > Bumping the version from 4.1.1 from 3.1.7, I ran in to
> > GfshCommandsOverHttpSecurityTest failing under geode-web.  The error
> was:
> >
> > [warn 2016/08/12 15:37:52.078 PDT  tid=0x1] loader constraint
> > violation: when resolving method "org.slf4j.impl.StaticLoggerBinder.
> > getLoggerFactory()Lorg/slf4j/ILoggerFactory;" the class loader
> (instance of
> > org/eclipse/jetty/webapp/WebAppClassLoader) of the current class,
> > org/slf4j/LoggerFactory, and the class loader (instance of
> > sun/misc/Launcher$AppClassLoader) for the method's defining class,
> > org/slf4j/impl/StaticLoggerBinder, have different Class objects for the
> > type org/slf4j/ILoggerFactory used in the signature
> > java.lang.LinkageError: loader constraint violation: when resolving
> method
> > "org.slf4j.impl.StaticLoggerBinder.getLoggerFactory()Lorg/slf4j/
> ILoggerFactory;"
> > the class loader (instance of org/eclipse/jetty/webapp/
> WebAppClassLoader)
> > of the current class, org/slf4j/LoggerFactory, and the class loader
> > (instance of sun/misc/Launcher$AppClassLoader) for the method's defining
> > class, org/slf4j/impl/StaticLoggerBinder, have different Class objects
> for
> > the type org/slf4j/ILoggerFactory used in the signature
>
>

Trying to upgrade spring-security

[Kevin Duling]

I'm working on adding security to the REST methods and wanted to start by
upgrading spring-security to the latest version.

Bumping the version from 4.1.1 from 3.1.7, I ran in to
GfshCommandsOverHttpSecurityTest failing under geode-web.  The error was:

[warn 2016/08/12 15:37:52.078 PDT  tid=0x1] loader constraint
violation: when resolving method "org.slf4j.impl.StaticLoggerBinder.
getLoggerFactory()Lorg/slf4j/ILoggerFactory;" the class loader (instance of
org/eclipse/jetty/webapp/WebAppClassLoader) of the current class,
org/slf4j/LoggerFactory, and the class loader (instance of
sun/misc/Launcher$AppClassLoader) for the method's defining class,
org/slf4j/impl/StaticLoggerBinder, have different Class objects for the
type org/slf4j/ILoggerFactory used in the signature
java.lang.LinkageError: loader constraint violation: when resolving method
"org.slf4j.impl.StaticLoggerBinder.getLoggerFactory()Lorg/slf4j/ILoggerFactory;"
the class loader (instance of org/eclipse/jetty/webapp/WebAppClassLoader)
of the current class, org/slf4j/LoggerFactory, and the class loader
(instance of sun/misc/Launcher$AppClassLoader) for the method's defining
class, org/slf4j/impl/StaticLoggerBinder, have different Class objects for
the type org/slf4j/ILoggerFactory used in the signature

Re: Swagger UI?

[Kevin Duling]

Perfect, thanks.  Exactly what I was looking for.

Actually, with GEODE-1467, it should be available at both /gemfire-api and
/geode for a time.  If it isn't, I need to revisit 1467.

On Tue, Aug 9, 2016 at 11:58 AM, Swapnil Bawaskar <sbawas...@pivotal.io>
wrote:

> with the work done for GEODE-1467, swagger API is now available at:
> /geode/docs/index.html
>
> On Tue, Aug 9, 2016 at 11:10 AM, Kevin Duling <kdul...@pivotal.io> wrote:
>
> > I saw that Geode has the annotations for Swagger.  How is the UI
> launched?
> > In projects I've worked with before, it's been deployed under the
> > "/swagger-ui" context.
> >
>

Swagger UI?

[Kevin Duling]

I saw that Geode has the annotations for Swagger.  How is the UI launched?
In projects I've worked with before, it's been deployed under the
"/swagger-ui" context.

Re: Review Request 50051: GEODE-1571: have auth-init accept either a constructor or a static factory method.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On July 14, 2016, 3:19 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50051/
> ---
> 
> (Updated July 14, 2016, 3:19 p.m.)
> 
> 
> Review request for geode, Grace Meilen, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1571: have auth-init accept either a constructor or a static factory 
> method.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  b82fdb1848d4ac0c0c47fe56f6699ba0dea84fde 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java
>  a2951f5f5a31d0ad80b3407097332067bf2f6e60 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java
>  8707a7895355cb187e9608b8fb6af2a33b36c2fa 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java
>  3d6275b58acdc94f94de2ed286bcb478c5b46352 
>   geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java 
> 400c6654bce587fdfca0789e33f4bc37b6064b8b 
>   geode-core/src/main/java/org/apache/geode/security/GeodePermission.java 
> 866b14ee591ed830387d87e6dcedab5ac6215393 
>   geode-core/src/main/java/org/apache/geode/security/PostProcessor.java 
> 0f13b47b5fc52ff176045db7d5a24f30d6158193 
>   
> geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java
>  7e078da19a5721a0a4bd79eb277b7384ecf47bd0 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtilTest.java
>  d1dd466b64630bbf92af8b70339ca73c8417b3cb 
> 
> Diff: https://reviews.apache.org/r/50051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 50141: GEODE-1571: simplify security check and have auth-init accept either a constructor or a static factory method.

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On July 18, 2016, 8:57 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50141/
> ---
> 
> (Updated July 18, 2016, 8:57 a.m.)
> 
> 
> Review request for geode, Grace Meilen, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1571: simplify security check
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  b82fdb1848d4ac0c0c47fe56f6699ba0dea84fde 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java
>  e93faf8957e385b7d4dbdc0ce7c78d360e1f4b63 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java
>  a2951f5f5a31d0ad80b3407097332067bf2f6e60 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java
>  8707a7895355cb187e9608b8fb6af2a33b36c2fa 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java
>  3d6275b58acdc94f94de2ed286bcb478c5b46352 
>   geode-core/src/main/java/com/gemstone/gemfire/security/AuthInitialize.java 
> 400c6654bce587fdfca0789e33f4bc37b6064b8b 
>   geode-core/src/main/java/org/apache/geode/security/GeodePermission.java 
> 866b14ee591ed830387d87e6dcedab5ac6215393 
>   geode-core/src/main/java/org/apache/geode/security/PostProcessor.java 
> 0f13b47b5fc52ff176045db7d5a24f30d6158193 
>   
> geode-core/src/main/java/org/apache/geode/security/templates/SamplePostProcessor.java
>  7e078da19a5721a0a4bd79eb277b7384ecf47bd0 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtilTest.java
>  d1dd466b64630bbf92af8b70339ca73c8417b3cb 
> 
> Diff: https://reviews.apache.org/r/50141/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 50462: GEODE-1385: mark testShutDownForTIMEOUT with FlakyTest category

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On July 26, 2016, 1:53 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50462/
> ---
> 
> (Updated July 26, 2016, 1:53 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Grace Meilen, Jinmei Liao, and Kevin 
> Duling.
> 
> 
> Bugs: GEODE-1385
> https://issues.apache.org/jira/browse/GEODE-1385
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1385: mark testShutDownForTIMEOUT with FlakyTest category
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/MiscellaneousCommandsDUnitTest.java
>  ab061d4 
> 
> Diff: https://reviews.apache.org/r/50462/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 50572: GEODE-1701: rename GeodePermission as ResourcePermission

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On July 28, 2016, 9:53 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50572/
> ---
> 
> (Updated July 28, 2016, 9:53 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, Grace Meilen, 
> Jinmei Liao, Kevin Duling, and Swapnil Bawaskar.
> 
> 
> Bugs: GEODE-1701
> https://issues.apache.org/jira/browse/GEODE-1701
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1701: rename GeodePermission as ResourcePermission
> 
> This is the same change we discussed on dev@geode list. I was preparing this 
> review when I accidentally pushed the change early, so it's already committed 
> to develop (sorry!). If anyone prefers that I back it out until we have a 
> couple positive reviews let me know. Otherwise I'll followup my commit with a 
> new commit to address any issues found during this review.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/cache/operations/OperationContext.java
>  f4d5280 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java
>  19f3325 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/shiro/CustomAuthRealm.java
>  c890dc9 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/AsyncEventQueueMXBean.java
>  093bee6 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/CacheServerMXBean.java
>  94dcbbb 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/DiskStoreMXBean.java 
> 484c43b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/DistributedLockServiceMXBean.java
>  26f785b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/DistributedRegionMXBean.java
>  4e384da 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/DistributedSystemMXBean.java
>  2cbb912 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/GatewayReceiverMXBean.java
>  9c7011c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/GatewaySenderMXBean.java
>  429e155 
>   geode-core/src/main/java/com/gemstone/gemfire/management/LocatorMXBean.java 
> c81267c 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/LockServiceMXBean.java
>  e255915 
>   geode-core/src/main/java/com/gemstone/gemfire/management/ManagerMXBean.java 
> 709b267 
>   geode-core/src/main/java/com/gemstone/gemfire/management/MemberMXBean.java 
> a3b036f 
>   geode-core/src/main/java/com/gemstone/gemfire/management/RegionMXBean.java 
> 397b1d9 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ClientCommands.java
>  83501ac 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommands.java
>  900f204 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java
>  3d49845 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DataCommands.java
>  89dce97 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DeployCommands.java
>  131775b 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DiskStoreCommands.java
>  a98bf80 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/DurableClientCommands.java
>  3f97e5d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java
>  268a1e4 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/FunctionCommands.java
>  cb61243 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
>  23d09dd 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/MemberCommands.java
>  c4e1b83 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/MiscellaneousCommands.java
>  ab5917d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/PDXCommands.java
>  ae8f36a 
>   
> geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/QueueCommands.java
&

Re: Renaming web api from gemfire-api to geode

[Kevin Duling]

I've followed Jinmei's suggestion and initial testing looks great.  I'm
going to rejigger existing unit tests to use parameters so it'll test both
geode and gemfire-api (as well as management), fire off a precheckin, and
see how it plays out.



On Tue, Jul 26, 2016 at 3:39 PM, Jinmei Liao <jil...@pivotal.io> wrote:

> It turns out in ManagementAgent or RestAgent, we are using JettyHelper to
> add the web apps under each specific context:
>
> JettyHelper.addWebApplication(httpServer, "/gemfire-api", gemfireAPIWar);
>
> So the solution is to add the same war file in the "/geode" context as
> well. This does mean that we will use more disk space since we installed
> two web apps instead of just one, but it's a quick solution, and we will
> delete the old web app after a few versions.
>
>
>
>
> On Tue, Jul 26, 2016 at 3:05 PM, Jens Deppe <jde...@pivotal.io> wrote:
>
> > Could you do a redirect instead of forwarding?
> >
> > --Jens
> >
> > > On Jul 26, 2016, at 1:57 PM, Kevin Duling <kdul...@pivotal.io> wrote:
> > >
> > > There is a story to rename the gemfire-api (Developer API) and gemfire
> > > (Management API) to geode and geode-mgmt, respectively.
> > >
> > > The change itself is rather trivial.  Just a couple web.xml changes,
> > rename
> > > gemfire-api-servlet.xml, and update unit tests.
> > >
> > > The non-trivial part is still supporting other applications that are
> > > expecting to access the old URLs.  The servlets are injected in to
> Jetty
> > > with a context, they don't just sit on the default / path.  As a
> result,
> > > I'm unable to do some aliasing in the servlet-mapping section of
> web.xml.
> > >
> > > How large of an impact is this going to be?
> >
>
>
>
> --
> Cheers
>
> Jinmei
>

Renaming web api from gemfire-api to geode

[Kevin Duling]

There is a story to rename the gemfire-api (Developer API) and gemfire
(Management API) to geode and geode-mgmt, respectively.

The change itself is rather trivial.  Just a couple web.xml changes, rename
gemfire-api-servlet.xml, and update unit tests.

The non-trivial part is still supporting other applications that are
expecting to access the old URLs.  The servlets are injected in to Jetty
with a context, they don't just sit on the default / path.  As a result,
I'm unable to do some aliasing in the servlet-mapping section of web.xml.

How large of an impact is this going to be?

Re: Valid characters in region names?

[Kevin Duling]

Do you know if a Jira ticket was opened on it?  I'm currently working on
https://issues.apache.org/jira/browse/GEODE-1615 and would like to link to
it.

On Thu, Jun 30, 2016 at 12:38 PM, Michael Stolz <mst...@pivotal.io> wrote:

> Yes we did discuss adding hyphen in Geode, and we discussed actually
> checking during create.
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: 631-835-4771
>
> On Thu, Jun 30, 2016 at 3:30 PM, Kevin Duling <kdul...@pivotal.io> wrote:
>
> > I found a thread from March of this year where this was discussed and '-'
> > is also included in that list.
> >
> > Shouldn't the creation of a region with an invalid character raise an
> > exception?  I'm finding that I can create and list regions with a variety
> > of non-alphanumeric characters.  But I can only destroy a subset of
> those.
> >
> >  It appears to be because some are created with quotes around the region
> > name.
> >
> > E.g.,
> >
> > service=Region, name=/good, type=Member
> > service=Region, name="/not*good", type=Member
> >
> > The name lookup fails in the Destroy call.  The rule that applied the
> > quotes for /not*good during Create isn't applied in Destroy
> >
> > On Thu, Jun 30, 2016 at 12:06 PM, Michael Stolz <mst...@pivotal.io>
> wrote:
> >
> > > The only characters that should be used in Region names are
> > > ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890_
> > >
> > > --
> > > Mike Stolz
> > > Principal Engineer, GemFire Product Manager
> > > Mobile: 631-835-4771
> > >
> > > On Thu, Jun 30, 2016 at 2:03 PM, Kevin Duling <kdul...@pivotal.io>
> > wrote:
> > >
> > > > I'm working on a bug where it is not possible to delete a region that
> > > has a
> > > > hyphen in it.  E.g., the region was created with:
> > > >
> > > > create region --name=not-good --type=REPLICATE
> > > >
> > > > I've a solution done, but see this as an opportunity to add a test
> for
> > > > other special characters.  Is there a list of valid and invalid
> > > characters
> > > > for a region name?
> > > >
> > >
> >
>

Re: Valid characters in region names?

[Kevin Duling]

I found a thread from March of this year where this was discussed and '-'
is also included in that list.

Shouldn't the creation of a region with an invalid character raise an
exception?  I'm finding that I can create and list regions with a variety
of non-alphanumeric characters.  But I can only destroy a subset of those.

 It appears to be because some are created with quotes around the region
name.

E.g.,

service=Region, name=/good, type=Member
service=Region, name="/not*good", type=Member

The name lookup fails in the Destroy call.  The rule that applied the
quotes for /not*good during Create isn't applied in Destroy

On Thu, Jun 30, 2016 at 12:06 PM, Michael Stolz <mst...@pivotal.io> wrote:

> The only characters that should be used in Region names are
> ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890_
>
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: 631-835-4771
>
> On Thu, Jun 30, 2016 at 2:03 PM, Kevin Duling <kdul...@pivotal.io> wrote:
>
> > I'm working on a bug where it is not possible to delete a region that
> has a
> > hyphen in it.  E.g., the region was created with:
> >
> > create region --name=not-good --type=REPLICATE
> >
> > I've a solution done, but see this as an opportunity to add a test for
> > other special characters.  Is there a list of valid and invalid
> characters
> > for a region name?
> >
>

Re: Review Request 49331: GEODE-1551: add ignoredExcpetions for CommandOverHttpDUnitTest

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On June 28, 2016, 11:11 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49331/
> ---
> 
> (Updated June 28, 2016, 11:11 a.m.)
> 
> 
> Review request for geode, Kirk Lund and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1551: add ignoredExcpetions for CommandOverHttpDUnitTest
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CliCommandTestBase.java
>  ab2ce627524dc1a3c99b4a3b2f5f1312a9f990bf 
> 
> Diff: https://reviews.apache.org/r/49331/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Creating & Destroying Regions programmatically

[Kevin Duling]

I'm trying to figure out how to exercise the CreateRegion and
DestoryRegion classes from a client in order to test authorization.  I
can't figure out what the API would be for a client to trigger the
cmdExecute() call on the server.

For example, I was able to hit it in ClearRegion by having the client
execute the Region.clear() method.  Here's a snippet from the unit
test:

SerializableRunnable clearAuthorized = new SerializableRunnable() {
  @Override
  public void run() {
Cache cache =
SecurityTestUtils.createCacheClient("authRegionUser", "1234567",
serverPort, SecurityTestUtils.NO_EXCEPTION);
final Region region = cache.getRegion(SecurityTestUtils.REGION_NAME);
region.clear();
cache.close();
  }
};
client2.invoke(clearAuthorized);

Re: Geode Portfile for Mac Ports

[Kevin Duling]

That's a good idea, Roman, so I looked in to it.  It doesn't appear that
the rest of the Apache projects follow that pattern.  Cassandra, Tomcat,
maven, etc. all omit apache in the package name in mac ports.

On Tue, May 17, 2016 at 5:36 PM, Roman Shaposhnik <ro...@shaposhnik.org>
wrote:

> Awesome work, but please consider calling it apache-geode.
>
> Thanks,
> Roman.
>
> On Tue, May 17, 2016 at 1:23 PM, Kevin Duling <kdul...@pivotal.io> wrote:
> > Using the geode.rb file for Homebrew as a template, I was able to build a
> > Portfile for macports.org.
> >
> > Does anyone have any feedback before I submit it to the macports repo?
> No
> > code changes were necessary and it is using the same binary image the
> > Homebrew version is.  I've tested it as far as "Geode in 5 minutes" takes
> > you.
> >
> > I classified it under 'databases' as that is where the cassandra port
> lives.
> >
> > The following is some output from running it.  It properly goes out and
> > locates a mirror to pull the tarball from.
> >
> >> kduling@kduling-mbpro:~/tmp/ports$ port info geode
> >> geode @1.0.0-incubating.M2 (databases)
> >> Description:  Apache Geode (incubating) is a data management
> >> platform that provides real-time, consistent access to data-intensive
> >> applications throughout widely distributed cloud
> >>   architectures.
> >> Homepage: https://geode.apache.org/
> >> Platforms:darwin
> >> License:  Apache-2
> >
> >
> >> kduling@kduling-mbpro:~/tmp/ports$ sudo port install geode
> >> --->  Fetching archive for geode
> >> --->  Attempting to fetch
> >> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> >> http://sea.us.packages.macports.org/macports/packages/geode
> >> --->  Attempting to fetch
> >> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> >> https://packages.macports.org/geode
> >> --->  Attempting to fetch
> >> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> >> http://mse.uk.packages.macports.org/sites/packages.macports.org/geode
> >> --->  Fetching distfiles for geode
> >> --->  Verifying checksums for geode
> >> --->  Extracting geode
> >> --->  Configuring geode
> >> --->  Building geode
> >> --->  Staging geode into destroot
> >> --->  Installing geode @1.0.0-incubating.M2_0
> >> --->  Activating geode @1.0.0-incubating.M2_0
> >> Please refer to http://geode.incubator.apache.org/docs/
> >> --->  Cleaning geode
> >> --->  Updating database of binaries
> >
> >
> >>
> >> kduling@kduling-mbpro:~/tmp/ports$ gfsh
> >> _ __
> >>/ _/ __/ __/ // /
> >>   / /  __/ /___  /_  / _  /
> >>  / /__/ / /  _/ / // /
> >> /__/_/  /__/_//_/v1.0.0-incubating.M2
> >> Monitor and Manage Apache Geode (incubating)
> >> gfsh>quit
> >> Exiting...
> >
> >
> >>
> >> kduling@kduling-mbpro:~/tmp/ports$ which gfsh
> >> /opt/local/bin/gfsh
> >> kduling@kduling-mbpro:~/tmp/ports$ sudo port uninstall geode
> >> --->  Deactivating geode @1.0.0-incubating.M2_0
> >> --->  Cleaning geode
> >> --->  Uninstalling geode @1.0.0-incubating.M2_0
> >> --->  Cleaning geode
> >> kduling@kduling-mbpro:~/tmp/ports$ which gfsh
> >> kduling@kduling-mbpro:~/tmp/ports$
>

Geode Portfile for Mac Ports

[Kevin Duling]

Using the geode.rb file for Homebrew as a template, I was able to build a
Portfile for macports.org.

Does anyone have any feedback before I submit it to the macports repo?  No
code changes were necessary and it is using the same binary image the
Homebrew version is.  I've tested it as far as "Geode in 5 minutes" takes
you.

I classified it under 'databases' as that is where the cassandra port
lives.

The following is some output from running it.  It properly goes out and
locates a mirror to pull the tarball from.

kduling@kduling-mbpro:*~/tmp/ports*$ port info geode
> geode @1.0.0-incubating.M2 (databases)
> Description:  Apache Geode (incubating) is a data management
> platform that provides real-time, consistent access to data-intensive
> applications throughout widely distributed cloud
>   architectures.
> Homepage: https://geode.apache.org/
> Platforms:darwin
> License:  Apache-2


kduling@kduling-mbpro:*~/tmp/ports*$ sudo port install geode
> --->  Fetching archive for geode
> --->  Attempting to fetch
> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> http://sea.us.packages.macports.org/macports/packages/geode
> --->  Attempting to fetch
> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> https://packages.macports.org/geode
> --->  Attempting to fetch
> geode-1.0.0-incubating.M2_0.darwin_15.noarch.tbz2 from
> http://mse.uk.packages.macports.org/sites/packages.macports.org/geode
> --->  Fetching distfiles for geode
> --->  Verifying checksums for geode
> --->  Extracting geode
> --->  Configuring geode
> --->  Building geode
> --->  Staging geode into destroot
> --->  Installing geode @1.0.0-incubating.M2_0
> --->  Activating geode @1.0.0-incubating.M2_0
> Please refer to http://geode.incubator.apache.org/docs/
> --->  Cleaning geode
> --->  Updating database of binaries
>


> kduling@kduling-mbpro:*~/tmp/ports*$ gfsh
> _ __
>/ _/ __/ __/ // /
>   / /  __/ /___  /_  / _  /
>  / /__/ / /  _/ / // /
> /__/_/  /__/_//_/v1.0.0-incubating.M2
> Monitor and Manage Apache Geode (incubating)
> gfsh>quit
> Exiting...
>


> kduling@kduling-mbpro:*~/tmp/ports*$ which gfsh
> /opt/local/bin/gfsh
> kduling@kduling-mbpro:*~/tmp/ports*$ sudo port uninstall geode
> --->  Deactivating geode @1.0.0-incubating.M2_0
> --->  Cleaning geode
> --->  Uninstalling geode @1.0.0-incubating.M2_0
> --->  Cleaning geode
> kduling@kduling-mbpro:*~/tmp/ports*$ which gfsh
> kduling@kduling-mbpro:*~/tmp/ports*$

Re: Review Request 47146: GEODE-1369: change ConfigCommandsDUnitTest to use TemporaryFolder

[Kevin Duling]

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


Ship it!




Ship It!

- Kevin Duling


On May 9, 2016, 5:30 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47146/
> ---
> 
> (Updated May 9, 2016, 5:30 p.m.)
> 
> 
> Review request for geode, Jens Deppe and Jinmei Liao.
> 
> 
> Bugs: GEODE-1369
> https://issues.apache.org/jira/browse/GEODE-1369
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1369: change ConfigCommandsDUnitTest to use TemporaryFolder
> 
> * use TemporaryFolder for all disk files
> * re-enable testAlterUpdatesSharedConfig (might be a FlakyTest) -- was 
> disabled for TRAC #52204 but it passes
> * use static imports
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/ConfigCommandsDUnitTest.java
>  c342142 
> 
> Diff: https://reviews.apache.org/r/47146/diff/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>