Re: Review Request 61506: GEODE-3328: refactor ConnectCommand

2017-08-08 Thread Jinmei Liao 

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

(Updated Aug. 9, 2017, 4:52 a.m.)


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


Repository: geode


Description
---

* connect command will prompt for missing ssl configs if ssl is implied.
* command ssl options will override the properties loaded in the file and 
prompt for missing ones if ssl is defined using the ssl-* prefix
* reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
Properties object


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 91a6443b8fd98874feb9f17cf15b36826d7982c3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 d4068910779c93b800d795d7f31f49a722ce6576 
  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 4b98617465d282fd9ff50cf551a66d4359b4111d 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38e4f893b0435d99b2192256fb559751b00 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
 16d45bc719817782d84ae7e3da876fb9ed4a77bb 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
 3e1357dad711c134cc45e5415b132dfdde92d93f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 1aea253ff72dfd2bd89754e862cf222754286a94 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
 5b289a545e2d324bfa516cf1b05f8df641bc8a36 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 a69ce36afa6762890edcfb58f48bd8eddd27be65 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
 9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
 31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
 d7db4897d58d318496c1ba990c792ba94f77c81e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
 PRE-CREATION 
  
geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
 2cd69ddc619a68fb151e6574bace7418a7d58d10 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
 e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 


Diff: https://reviews.apache.org/r/61506/diff/2/

Changes: https://reviews.apache.org/r/61506/diff/1-2/


Testing
---

precheckin running


Thanks,

Jinmei Liao

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

2017-08-08 Thread pivotal-amurmann 
GitHub user pivotal-amurmann opened a pull request:

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

Mark ProtoBuf interface as experimental

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced in 
the commit message?
3042

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

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

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?
N/A

- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
N/A

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build 
issues and
submit an update to your PR as soon as possible. If you need help, please 
send an
email to dev@geode.apache.org.


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

$ git pull https://github.com/pivotal-amurmann/geode chore/GEODE-3402

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

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

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

This closes #698


commit 26425bb0ce0cb0936fe0c0534abf5f1f5eac0e05
Author: Bruce Schuchardt 
Date:   2017-08-08T23:29:29Z

Mark ProtoBuf interface as experimental

Signed-off-by: Alexander Murmann 




---
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.
---

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #641 was SUCCESSFUL (with 2027 tests). Change made by John Blum.

2017-08-08 Thread Spring CI 

---
Spring Data GemFire > Nightly-ApacheGeode > #641 was successful.
---
Scheduled with changes by John Blum.
2029 tests in total.

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




--
Code Changes
--
John Blum (9530287e055acbfbbfdf6f570af4a793fa465943):

>DATAGEODE-35 - Add missing configuration support for 
>critical-off-heap-percentage and eviction-off-heap-percentage.



--
This message is automatically generated by Atlassian Bamboo

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Ken Howe 

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


Ship it!




Ship It!

- Ken Howe


On Aug. 8, 2017, 9:10 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 8, 2017, 9:10 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQ.java
>  a3d51edcc141391e9d818fc0ed7e514d3cb5d6d0 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQTest.java
>  PRE-CREATION 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java
>  cc5dde409c561522ae3739eeaee892079c509ae8 
>   
> geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jared Stewart 

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


Ship it!




Ship It!

- Jared Stewart


On Aug. 8, 2017, 9:10 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 8, 2017, 9:10 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQ.java
>  a3d51edcc141391e9d818fc0ed7e514d3cb5d6d0 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQTest.java
>  PRE-CREATION 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java
>  cc5dde409c561522ae3739eeaee892079c509ae8 
>   
> geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

[GitHub] geode issue #664: Fix for GEODE-3292

2017-08-08 Thread jinmeiliao 
Github user jinmeiliao commented on the issue:

https://github.com/apache/geode/pull/664
  
I will pull this in the moment I got a green pipeline


---
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 61420: GEODE-3307 CI failure: Uncaught exception in thread Thread[Geode Membership View Creator

2017-08-08 Thread Udo Kohlmeyer 

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


Ship it!




Ship It!

- Udo Kohlmeyer


On Aug. 3, 2017, 10:07 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61420/
> ---
> 
> (Updated Aug. 3, 2017, 10:07 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Now we catch DistributedSystemDisconnectedException in ViewCreatoe thread and 
> then mark shutdown true
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  40a4254 
> 
> 
> Diff: https://reviews.apache.org/r/61420/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Ken Howe 

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


Ship it!




- Ken Howe


On Aug. 8, 2017, 9:10 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 8, 2017, 9:10 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQ.java
>  a3d51edcc141391e9d818fc0ed7e514d3cb5d6d0 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQTest.java
>  PRE-CREATION 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java
>  cc5dde409c561522ae3739eeaee892079c509ae8 
>   
> geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jinmei Liao 


> On Aug. 7, 2017, 8:21 p.m., Jared Stewart wrote:
> > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
> > Lines 140 (patched)
> > 
> >
> > Can you clarify something for me?  The change looks like it's addding a 
> > check for CLUSTER:MANAGE:QUERY.  But from reading GEODE-3330, I thought we 
> > already checked for that and instead needed to add a check for 
> > DATA:READ:REGION.  But it looks like that might have already been happening 
> > on line 126 (122 of the original code).
> 
> Jinmei Liao wrote:
> line 126 is the authorization for old AccessControl interface, it does 
> nothing for the new integrated security. it was there in case user is still 
> using the old security model.
> 
> I was orignally confused as well. Turns out we are checking 
> DATA:READ:regionName already for both execute and executeWithInitialResult, 
> we are just adding the CLUSTER:MANAGE.QUERY check.
> 
> Jared Stewart wrote:
> Would you mind pointing me to where the existing check happens for my own 
> edification?  Thanks!
> 
> Jinmei Liao wrote:
> It's in line 184 processQuery, which eventually leads to BaseCommandQuery 
> line 90.
> 
> Jared Stewart wrote:
> Good to know.  Thanks again.
> 
> Ken Howe wrote:
> My understanding was that DATA:READ:regionName was to be required only 
> for executeWithInitialResult, not for execute. As it is now this change 
> requires both permissions for both execute and executeWithInitialResult. Is 
> this what we want?

Yes, according to Dan. He said the user who calls execute() will get the result 
back in CQEvents for updates of the data, so whether or not he/she will get the 
inital set of result back or not, he will get continuous update of the data 
afterwards.


- Jinmei


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


On Aug. 8, 2017, 9:10 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 8, 2017, 9:10 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQ.java
>  a3d51edcc141391e9d818fc0ed7e514d3cb5d6d0 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/StopCQTest.java
>  PRE-CREATION 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java
>  cc5dde409c561522ae3739eeaee892079c509ae8 
>   
> geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61506: GEODE-3328: refactor ConnectCommand

2017-08-08 Thread Udo Kohlmeyer 

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


Fix it, then Ship it!




Other than the SSLConfigurationFactory changes, I cannot comment on the 
validity of the other code.


geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
Lines 68-69 (patched)


Maybe this become @deprecated from GEODE 1.3 onwards. use


- Udo Kohlmeyer


On Aug. 8, 2017, 7:30 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61506/
> ---
> 
> (Updated Aug. 8, 2017, 7:30 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, 
> Patrick Rhomberg, and Udo Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * connect command will prompt for missing ssl configs if ssl is implied.
> * command ssl options will override the properties loaded in the file and 
> prompt for missing ones if ssl is defined using the ssl-* prefix
> * reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
> Properties object
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
>  91a6443b8fd98874feb9f17cf15b36826d7982c3 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
>  d4068910779c93b800d795d7f31f49a722ce6576 
>   
> geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
>  4b98617465d282fd9ff50cf551a66d4359b4111d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
>  eb71d38e4f893b0435d99b2192256fb559751b00 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
>  16d45bc719817782d84ae7e3da876fb9ed4a77bb 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
>  3e1357dad711c134cc45e5415b132dfdde92d93f 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
>  be556a447d862144e453f69809a2de67ee00b654 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  1aea253ff72dfd2bd89754e862cf222754286a94 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
>  5b289a545e2d324bfa516cf1b05f8df641bc8a36 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  a69ce36afa6762890edcfb58f48bd8eddd27be65 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
>  7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
>  9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
>   
> geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
>  31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
>   
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
>  866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
>   
> geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
>  d7db4897d58d318496c1ba990c792ba94f77c81e 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
>  PRE-CREATION 
>   
> geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
>  2cd69ddc619a68fb151e6574bace7418a7d58d10 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
>  PRE-CREATION 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
>  PRE-CREATION 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
>  PRE-CREATION 
>   
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
>  e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 
> 
> 
> Diff: https://reviews.apache.org/r/61506/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Review Request 61507: GEODE-3313: Test utility supports building jar files with multiple classes

2017-08-08 Thread Jared Stewart 

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

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


Repository: geode


Description
---

GEODE-3383: Refactor deploy tests

- Remove some duplicated tests
- Re-organize some tests
- DeployedJarJUnitTest now tests the public api of that class rather than 
implementation details


Diffs
-

  
geode-core/src/test/java/org/apache/geode/internal/ClassPathLoaderIntegrationTest.java
 34d8a2318422edbb3bbdc04920a41693f8d3fe69 
  geode-core/src/test/java/org/apache/geode/internal/DeployedJarJUnitTest.java 
178dbae2eaada0cc054502fdf4b6d2ff102b4ed6 
  
geode-core/src/test/java/org/apache/geode/internal/JarDeployerDeadlockTest.java 
PRE-CREATION 
  geode-core/src/test/java/org/apache/geode/management/DeployJarTestSuite.java 
6dfab66926c7b246bf839632b293330959f1d728 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
 89148d7752ae1f69e25671ffc43101a63cf7dc64 
  geode-junit/build.gradle e47095f5c538c4ae040b7135bf20eeef55bd77ba 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/ClassNameExtractor.java
 PRE-CREATION 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/CompiledSourceCode.java
 PRE-CREATION 
  geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java 
PRE-CREATION 
  geode-junit/src/main/java/org/apache/geode/test/compiler/JavaCompiler.java 
PRE-CREATION 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/UncompiledSourceCode.java
 PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/ClassNameExtractorTest.java
 PRE-CREATION 
  geode-junit/src/test/java/org/apache/geode/test/compiler/JarBuilderTest.java 
PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/JavaCompilerTest.java 
PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/UncompiledSourceCodeTest.java
 PRE-CREATION 
  
geode-junit/src/test/resources/org/apache/geode/test/compiler/AbstractClass.java
 PRE-CREATION 
  
geode-junit/src/test/resources/org/apache/geode/test/compiler/ConcreteClass.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpDUnitTest.java
 7753aafbd7dc5ea4288e27f088400163f6739347 


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


Testing
---


Thanks,

Jared Stewart

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jared Stewart 

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




geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java
Lines 54 (patched)


Oops, I was a little trigger happy with my "Ship it!".. Should there be one 
more test here to make sure that things work as expected when a user has both 
permissions?


- Jared Stewart


On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jared Stewart 

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


Ship it!




Ship It!

- Jared Stewart


On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jinmei Liao 


> On Aug. 7, 2017, 8:21 p.m., Jared Stewart wrote:
> > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
> > Lines 140 (patched)
> > 
> >
> > Can you clarify something for me?  The change looks like it's addding a 
> > check for CLUSTER:MANAGE:QUERY.  But from reading GEODE-3330, I thought we 
> > already checked for that and instead needed to add a check for 
> > DATA:READ:REGION.  But it looks like that might have already been happening 
> > on line 126 (122 of the original code).
> 
> Jinmei Liao wrote:
> line 126 is the authorization for old AccessControl interface, it does 
> nothing for the new integrated security. it was there in case user is still 
> using the old security model.
> 
> I was orignally confused as well. Turns out we are checking 
> DATA:READ:regionName already for both execute and executeWithInitialResult, 
> we are just adding the CLUSTER:MANAGE.QUERY check.
> 
> Jared Stewart wrote:
> Would you mind pointing me to where the existing check happens for my own 
> edification?  Thanks!

It's in line 184 processQuery, which eventually leads to BaseCommandQuery line 
90.


- Jinmei


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


On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Gitbox enables the full GitHub workflow

2017-08-08 Thread Mark Bretl 
+1 for Gitbox

I have been watching threads on INFRA list and been waiting to see when a
good time would be to introduce the idea to the community. The Gitbox
project has been going since the end of 2016 and looks as though it might
be ready now as INFRA has begun moving some of their repositories to
GitHub. We still need to 'apply' by submitting a ticket to INFRA and wait
for approval, INFRA could still deny the move.

Advantages:
- GitHub PRs are fully functional (something which stopped us earlier)
- Provides ability to align committer and non-committer workflows
- Removing mirror from Git at Apache to GitHub and thus removing delay in
sync

Disadvantages:
- Permission mapping is configured on id.apache.org (need to set GitHub ID)

I can't think of many disadvantages to moving to GitBox.

--Mark


On Tue, Aug 8, 2017 at 10:56 AM, Dan Smith  wrote:

> I'm in favor of switching to gitbox. We're already getting pretty much all
> contributions from non-committers coming in as pull requests so we might as
> well make it easier for us to manage things in github.
>
> -Dan
>
> On Tue, Aug 8, 2017 at 8:57 AM, Kirk Lund  wrote:
>
> > One thing that's given me trouble is that the mirroring of ASF git (for
> > Apache Geode) to github can have a lengthy delay. In other words, after I
> > commit to ASF git, it's not uncommon for my commit to not show up in the
> > GitHub mirror right away and I've seen this delay take an hour.
> >
> > On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik 
> > wrote:
> >
> > > Hi!
> > >
> > > it has just come to my attention that Gitbox at ASF
> > > has been enabling full GitHub workflow (with being
> > > able to click Merge this PR button, etc.) for quite
> > > some time.
> > >
> > > This basically allows a project to have GH as a R/W
> > > repo as opposed to R/O mirror of what we all currnently
> > > have: https://gitbox.apache.org/repos/asf
> > >
> > > Personally I'm not sure I like GH workflow all that much,
> > > but if there's interest -- you can opt-in into Gitbox. Once
> > > you do -- your source of truth moves to GH. You can't
> > > have it both ways with git-wip-us.apache.org and Gitbox.
> > >
> > > Thanks,
> > > Roman.
> > >
> >
>

Review Request 61506: GEODE-3328: refactor ConnectCommand

2017-08-08 Thread Jinmei Liao 

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

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


Repository: geode


Description
---

* connect command will prompt for missing ssl configs if ssl is implied.
* command ssl options will override the properties loaded in the file and 
prompt for missing ones if ssl is defined using the ssl-* prefix
* reworked the SSLConfigFactory to not use cached sslConfig when passing in a 
Properties object


Diffs
-

  
geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
 91a6443b8fd98874feb9f17cf15b36826d7982c3 
  
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpClient.java
 d4068910779c93b800d795d7f31f49a722ce6576 
  
geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java
 4b98617465d282fd9ff50cf551a66d4359b4111d 
  
geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerLocatorRequest.java
 eb71d38e4f893b0435d99b2192256fb559751b00 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java
 16d45bc719817782d84ae7e3da876fb9ed4a77bb 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/AbstractCliAroundInterceptor.java
 3e1357dad711c134cc45e5415b132dfdde92d93f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
 be556a447d862144e453f69809a2de67ee00b654 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
 1aea253ff72dfd2bd89754e862cf222754286a94 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/StartLocatorCommand.java
 5b289a545e2d324bfa516cf1b05f8df641bc8a36 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UserInputProperty.java
 PRE-CREATION 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 58c8ef79c70f789d10d0a6e95cb24b510aa014a0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
 a69ce36afa6762890edcfb58f48bd8eddd27be65 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
 7ae7c3b0f7d85aae1a8d16ba0fa7d9b80683bdfd 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/util/GfshConsoleReader.java
 9251d7e127a085e3bdb9bfcffc6c706b50b042dd 
  
geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
 31c2469a6ef7d9a1db9f2803b0b5bd9adfb5971c 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java
 866f4ef43dd1c583df210caa491cdd0ef8a3b84a 
  
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorWithLegacySSLDUnitTest.java
 d7db4897d58d318496c1ba990c792ba94f77c81e 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/UserInputPropertyTest.java
 PRE-CREATION 
  
geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java
 2cd69ddc619a68fb151e6574bace7418a7d58d10 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandIntegrationTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSSLTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandWithSecurityTest.java
 PRE-CREATION 
  
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectToLocatorSSLOverHttpTest.java
 e5b8d25ae4349e19c252c4d364d0a3c50edeeb54 


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


Testing
---

precheckin running


Thanks,

Jinmei Liao

Re: Review Request 61420: GEODE-3307 CI failure: Uncaught exception in thread Thread[Geode Membership View Creator

2017-08-08 Thread Alexander Murmann 

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


Ship it!





geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
Lines 2083 (patched)


since we do the same thing in both cases why not do
```
catch (InterruptedException | DistributedSystemDisconnectedException e) {...
```


- Alexander Murmann


On Aug. 3, 2017, 10:07 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61420/
> ---
> 
> (Updated Aug. 3, 2017, 10:07 p.m.)
> 
> 
> Review request for geode, Alexander Murmann, Bruce Schuchardt, Galen 
> O'Sullivan, Udo Kohlmeyer, and Brian Rowe.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Now we catch DistributedSystemDisconnectedException in ViewCreatoe thread and 
> then mark shutdown true
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  40a4254 
> 
> 
> Diff: https://reviews.apache.org/r/61420/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

[GitHub] geode pull request #683: GEODE-3314 - additional refactoring for developer Q...

2017-08-08 Thread pivotal-amurmann 
Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/683#discussion_r131974883
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
 ---
@@ -74,17 +73,6 @@
   public static final long NOT_GRANTOR_SLEEP = Long
   .getLong(DistributionConfig.GEMFIRE_PREFIX + 
"DLockService.notGrantorSleep", 100).longValue();
 
-  public static final boolean DEBUG_DISALLOW_NOT_HOLDER = Boolean
-  .getBoolean(DistributionConfig.GEMFIRE_PREFIX + 
"DLockService.debug.disallowNotHolder");
-
-  public static final boolean DEBUG_LOCK_REQUEST_LOOP = Boolean
-  .getBoolean(DistributionConfig.GEMFIRE_PREFIX + 
"DLockService.debug.disallowLockRequestLoop");
-
-  public static final int DEBUG_LOCK_REQUEST_LOOP_COUNT = Integer
-  .getInteger(
-  DistributionConfig.GEMFIRE_PREFIX + 
"DLockService.debug.disallowLockRequestLoopCount", 20)
-  .intValue();
-
   public static final boolean DEBUG_NONGRANTOR_DESTROY_LOOP = Boolean
--- End diff --

how comes we are keeping this debug harness but not the others?


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

[GitHub] geode pull request #683: GEODE-3314 - additional refactoring for developer Q...

2017-08-08 Thread pivotal-amurmann 
Github user pivotal-amurmann commented on a diff in the pull request:

https://github.com/apache/geode/pull/683#discussion_r131977004
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
 ---
@@ -1433,29 +1412,7 @@ public boolean lockInterruptibly(final Object name, 
final long waitTimeMillis,
 int lockId = -1;
 incActiveLocks();
 
-int loopCount = 0;
 while (keepTrying) {
--- End diff --

This `keepTrying` thing is very unclear because it hides what the actual 
decision is based on. It seems like we keep trying until either the `waitLimit` 
has been exceeded or the lock has been acquired. Could we split `keepTrying` 
out into `boolean lockAcquired` and `boolean waitLimitExceeded`? Maybe the 
check for `waitTimeLimit` doesn't even need a variable but could be checked by 
the `while`?


---
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 61166: GEODE-3313: Test utility supports building jar files with multiple classes

2017-08-08 Thread Jared Stewart 

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

(Updated Aug. 8, 2017, 5:14 p.m.)


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


Changes
---

Improve error handling and remove unused test resources.


Repository: geode


Description
---

GEODE-3313: Test utility supports building jar files with multiple classes


Diffs (updated)
-

  geode-junit/build.gradle e47095f 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/ClassNameExtractor.java
 PRE-CREATION 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/CompiledSourceCode.java
 PRE-CREATION 
  geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java 
PRE-CREATION 
  geode-junit/src/main/java/org/apache/geode/test/compiler/JavaCompiler.java 
PRE-CREATION 
  
geode-junit/src/main/java/org/apache/geode/test/compiler/UncompiledSourceCode.java
 PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/ClassNameExtractorTest.java
 PRE-CREATION 
  geode-junit/src/test/java/org/apache/geode/test/compiler/JarBuilderTest.java 
PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/JavaCompilerTest.java 
PRE-CREATION 
  
geode-junit/src/test/java/org/apache/geode/test/compiler/UncompiledSourceCodeTest.java
 PRE-CREATION 
  
geode-junit/src/test/resources/org/apache/geode/test/compiler/AbstractClass.java
 PRE-CREATION 
  
geode-junit/src/test/resources/org/apache/geode/test/compiler/ConcreteClass.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/61166/diff/8/

Changes: https://reviews.apache.org/r/61166/diff/7-8/


Testing
---

Precheckin running


Thanks,

Jared Stewart

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

2017-08-08 Thread Jared Stewart 


> On Aug. 7, 2017, 8:21 p.m., Jared Stewart wrote:
> > geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
> > Lines 140 (patched)
> > 
> >
> > Can you clarify something for me?  The change looks like it's addding a 
> > check for CLUSTER:MANAGE:QUERY.  But from reading GEODE-3330, I thought we 
> > already checked for that and instead needed to add a check for 
> > DATA:READ:REGION.  But it looks like that might have already been happening 
> > on line 126 (122 of the original code).
> 
> Jinmei Liao wrote:
> line 126 is the authorization for old AccessControl interface, it does 
> nothing for the new integrated security. it was there in case user is still 
> using the old security model.
> 
> I was orignally confused as well. Turns out we are checking 
> DATA:READ:regionName already for both execute and executeWithInitialResult, 
> we are just adding the CLUSTER:MANAGE.QUERY check.

Would you mind pointing me to where the existing check happens for my own 
edification?  Thanks!


- Jared


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


On Aug. 7, 2017, 8:13 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> ---
> 
> (Updated Aug. 7, 2017, 8:13 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
> Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * cq.execute() and cq.executeWithInitialResult() all would still require 
> DATA:READ because it will send the result back to the client either initially 
> or later.
> 
> 
> Diffs
> -
> 
>   geode-core/src/test/java/org/apache/geode/security/SecurityTestUtil.java 
> 5d5c2148e2eba8054df305942e04f43ea69c0a79 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  a455aff315cd26abd398630ff63d8b54b0d1d12b 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
>  6748f7d3193babfa668a7ff2846f974f0cdc1cbd 
>   
> geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
>  77a608c7e65a2030c0037e9f327cf8c17e9313db 
>   geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java 
> 89167258ddbc06315068c849255a8530faefe060 
>   
> geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java
>  45f45abd17cf4f90f96163ebe4f01e67b3b53633 
>   geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61480/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-08 Thread Jared Stewart 

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


Ship it!




Ship It!

- Jared Stewart


On Aug. 8, 2017, 4:38 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61487/
> ---
> 
> (Updated Aug. 8, 2017, 4:38 p.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3407
> https://issues.apache.org/jira/browse/GEODE-3407
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Change InternalClientMembership to not synchronize on CacheFactory
> by accepting Cache parameter from CacheServerBridge.
> 
> New regression test confirms bug and this fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
>  504081d65515adb294dd43ecffee450477f08339 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
>  728402c8a290d88026db753657e26e5f7440a990 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java
>  003a8f3cca4ff8fab031bdd84e0e360a14334f6b 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
>  6834998da13deec5e074a61e5373ec2f8dce2ad7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61487/diff/2/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

[GitHub] geode pull request #668: GEODE-3306: Remove whitespace StringBuffers/nodes c...

2017-08-08 Thread jaredjstewart 
Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/668#discussion_r131968121
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
 ---
@@ -111,10 +113,31 @@ public void testCacheXmlParserWithSimplePool() {
 InternalDistributedSystem system =
 InternalDistributedSystem.newInstanceForTesting(dm, nonDefault);
 when(dm.getSystem()).thenReturn(system);
-InternalDistributedSystem.connect(nonDefault);
 
-CacheXmlParser.parse(getClass()
-
.getResourceAsStream("CacheXmlParserJUnitTest.testSimpleClientCacheXml.cache.xml"));
+Cache cache = new CacheFactory()
+.set("cache-xml-file", 
"CacheXmlParserJUnitTest.testSimpleClientCacheXml.cache.xml")
+.create(InternalDistributedSystem.connect(nonDefault));
+cache.close();
+  }
+
+  /**
+   * Test that {@link CacheXmlParser} can parse the test cache.xml file, 
using the Apache Xerces XML
+   * parser.
+   * 
+   * @since Geode 1.3
+   */
+  @Test
+  public void testCacheXmlParserWithSimplePoolXerces() {
+String prevParserFactory = 
System.setProperty("javax.xml.parsers.SAXParserFactory",
+"org.apache.xerces.jaxp.SAXParserFactoryImpl");
+
+testCacheXmlParserWithSimplePool();
+
+if (prevParserFactory != null) {
--- End diff --

This cleanup of system properties can be made simpler by using the 
`RestoreSystemProperties` JUnit rule.  All you need to do is add this member 
variable to your test class: 

```  @Rule
  public final RestoreSystemProperties restoreSystemProperties = new 
RestoreSystemProperties();
```

(For an example, see 
[LocatorLauncherTest](https://github.com/apache/geode/blob/d1db2f02d2ce45a437b34488934e5b1d53c7b5ca/geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java).)


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

[GitHub] geode pull request #668: GEODE-3306: Remove whitespace StringBuffers/nodes c...

2017-08-08 Thread jaredjstewart 
Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/668#discussion_r131967266
  
--- Diff: 
geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java
 ---
@@ -2596,6 +2596,18 @@ private void endDeclarable() {
 
   public void startElement(String namespaceURI, String localName, String 
qName, Attributes atts)
   throws SAXException {
+// This while loop pops all StringBuffers at the top of the stack
+// that contain only whitespace; see GEODE-3306
+while (!stack.empty()) {
+  Object o = stack.peek();
+  if (o instanceof StringBuffer
+  && ((StringBuffer) o).toString().replaceAll("\\s", 
"").equals("")) {
--- End diff --

I think `StringUtils.isBlank( (StringBuffer o).toString())` might be 
simpler here as well as in `endElement`.


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

[GitHub] geode pull request #668: GEODE-3306: Remove whitespace StringBuffers/nodes c...

2017-08-08 Thread jaredjstewart 
Github user jaredjstewart commented on a diff in the pull request:

https://github.com/apache/geode/pull/668#discussion_r131966460
  
--- Diff: 
geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
 ---
@@ -128,9 +128,16 @@ public void testCacheXmlParserWithSimplePool() {
*/
   @Test
   public void testCacheXmlParserWithSimplePoolXerces() {
-System.setProperty("javax.xml.parsers.SAXParserFactory",
+String prevParserFactory = 
System.setProperty("javax.xml.parsers.SAXParserFactory",
--- End diff --

You can make this cleanup of the system properties more robust by using the 
`RestoreSystemProperties` JUnit rule.  All you need to do is add this as a 
member variable of the test class: 
```
  @Rule
  public final RestoreSystemProperties restoreSystemProperties = new 
RestoreSystemProperties();
```

(For an example see 
[LocatorLauncherTest](https://github.com/apache/geode/blob/d1db2f02d2ce45a437b34488934e5b1d53c7b5ca/geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java).)


---
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 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-08 Thread Kirk Lund 

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

(Updated Aug. 8, 2017, 4:38 p.m.)


Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, and 
Patrick Rhomberg.


Bugs: GEODE-3407
https://issues.apache.org/jira/browse/GEODE-3407


Repository: geode


Description
---

Change InternalClientMembership to not synchronize on CacheFactory
by accepting Cache parameter from CacheServerBridge.

New regression test confirms bug and this fix.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
 504081d65515adb294dd43ecffee450477f08339 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
 728402c8a290d88026db753657e26e5f7440a990 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/ManagementAdapter.java
 003a8f3cca4ff8fab031bdd84e0e360a14334f6b 
  
geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
 6834998da13deec5e074a61e5373ec2f8dce2ad7 
  
geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/61487/diff/2/

Changes: https://reviews.apache.org/r/61487/diff/1-2/


Testing
---

precheckin in progress


Thanks,

Kirk Lund

Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-08 Thread Kirk Lund 


> On Aug. 8, 2017, 1:04 a.m., Jared Stewart wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
> > Line 292 (original), 292 (patched)
> > 
> >
> > I might be missing something, but would callers of the no-arg variant 
> > `getClientQueueSizes()` potentially be prone to the same deadlock that 
> > originally affected `CacheServerBridge.getNumSubscriptions`?

The other callers are a Hydra test and the deprecated Admin API. If I change 
those then all I can do is move CacheFactory.getAnyInstance to the caller which 
doesn't seem worth the effort.


> On Aug. 8, 2017, 1:04 a.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
> > Lines 91 (patched)
> > 
> >
> > I really like this style of this test!

Thanks!


- Kirk


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


On Aug. 8, 2017, 12:19 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61487/
> ---
> 
> (Updated Aug. 8, 2017, 12:19 a.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3407
> https://issues.apache.org/jira/browse/GEODE-3407
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Change InternalClientMembership to not synchronize on CacheFactory
> by accepting Cache parameter from CacheServerBridge.
> 
> New regression test confirms bug and this fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
>  504081d65515adb294dd43ecffee450477f08339 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
>  728402c8a290d88026db753657e26e5f7440a990 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
>  6834998da13deec5e074a61e5373ec2f8dce2ad7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61487/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Gitbox enables the full GitHub workflow

2017-08-08 Thread Kirk Lund 
One thing that's given me trouble is that the mirroring of ASF git (for
Apache Geode) to github can have a lengthy delay. In other words, after I
commit to ASF git, it's not uncommon for my commit to not show up in the
GitHub mirror right away and I've seen this delay take an hour.

On Mon, Aug 7, 2017 at 6:09 PM, Roman Shaposhnik 
wrote:

> Hi!
>
> it has just come to my attention that Gitbox at ASF
> has been enabling full GitHub workflow (with being
> able to click Merge this PR button, etc.) for quite
> some time.
>
> This basically allows a project to have GH as a R/W
> repo as opposed to R/O mirror of what we all currnently
> have: https://gitbox.apache.org/repos/asf
>
> Personally I'm not sure I like GH workflow all that much,
> but if there's interest -- you can opt-in into Gitbox. Once
> you do -- your source of truth moves to GH. You can't
> have it both ways with git-wip-us.apache.org and Gitbox.
>
> Thanks,
> Roman.
>

Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

2017-08-08 Thread Ken Howe 

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




geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
Lines 114-115 (patched)


To avoid any confusion, I suggest that the order of the cache and 
cacheServer parameters be the same for this and the preceding constructor. I 
prefer the order decalred here, (cache, cacheServer, ...)


- Ken Howe


On Aug. 8, 2017, 12:19 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61487/
> ---
> 
> (Updated Aug. 8, 2017, 12:19 a.m.)
> 
> 
> Review request for geode, Emily Yeh, Jinmei Liao, Jared Stewart, Ken Howe, 
> and Patrick Rhomberg.
> 
> 
> Bugs: GEODE-3407
> https://issues.apache.org/jira/browse/GEODE-3407
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Change InternalClientMembership to not synchronize on CacheFactory
> by accepting Cache parameter from CacheServerBridge.
> 
> New regression test confirms bug and this fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/tier/InternalClientMembership.java
>  504081d65515adb294dd43ecffee450477f08339 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
>  728402c8a290d88026db753657e26e5f7440a990 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/beans/ServerBridge.java
>  6834998da13deec5e074a61e5373ec2f8dce2ad7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/beans/CacheServerBridgeClientMembershipRegressionTest.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61487/diff/1/
> 
> 
> Testing
> ---
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

RE: Need to know complete execution time for geode-core Junit tests

2017-08-08 Thread Dinesh Akhand 
Thanks Darrel It works on Linux.
But there I see another failure:

:geode-core:distributedTest

org.apache.geode.internal.cache.FixedPRSinglehopDUnitTest > 
testMetadataInClientWithFixedPartitions FAILED
java.lang.AssertionError: bucket 3 was incorrect expected:<20189> but 
was:<29785>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at 
org.apache.geode.internal.cache.FixedPRSinglehopDUnitTest.testMetadataInClientWithFixedPartitions(FixedPRSinglehopDUnitTest.java:352)

Which all test are part of Geode CI .  only Junit ? or Integration ? 
distributedTest?

Thanks,
Dinesh Akhand

-Original Message-
From: Darrel Schneider [mailto:dschnei...@pivotal.io] 
Sent: Monday, August 7, 2017 9:59 PM
To: dev@geode.apache.org
Subject: Re: Need to know complete execution time for geode-core Junit tests

For test004StartServerFailsFastOnMissingGemFirePropertiesFile I think this test 
just needs to be improved for Windows.
The test has an assertion that the failure message contains a certain string. 
On Windows the message contains "C:" and "\" which the test does not expect. 
Instead of the test treating "cacheXmlPathName" as a constant I think it should 
create an instance of File using cacheXmlPathName and then make sure the 
message contains the result of calling getAbsolutePath on that File.


On Sun, Aug 6, 2017 at 11:54 PM, Dinesh Akhand  wrote:

> Hi Team,
>
> When we are running test on windows : we are getting below error We 
> ran : gradlew precheckin
>
>
> rg.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest > 
> test003StartServerFailsFastOnMissingCacheXmlFile
> FAILED
> java.lang.AssertionError:
> Warning: The Geode cache XML file C:\path\to\missing\cache.xml 
> could not be found.
>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.assertTrue(Assert.java:41)
> at org.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest.test003StartServerFailsFastOnM
> issingCacheXmlFile(LauncherLifecycleCommandsDUnitTest.java:502)
>
> org.apache.geode.management.internal.cli.commands.
> LauncherLifecycleCommandsDUnitTest > 
> test004StartServerFailsFastOnMissingGemFirePropertiesFile
> FAILED
> java.lang.AssertionError:
> Warning: The Geode properties file 
> C:\path\to\missing\gemfire.properties
> could not be found.
>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.assertTrue(Assert.java:41)
> at
>
> Is there any path we need to set in windows to make it run.
>
> Thanks,
> Dinesh Akhand
>
>
>
>
>
>
>
>
>
>
>
> 
> ---
> Found suspect string in log4j at line 546
>
> -Original Message-
> From: Dan Smith [mailto:dsm...@pivotal.io]
> Sent: Tuesday, July 25, 2017 9:59 PM
> To: dev@geode.apache.org
> Subject: Re: Need to know complete execution time for geode-core Junit 
> tests
>
> What target are you running? It's a bit confusing because there are 
> actually 3 different sets of tests. These times a really rough because 
> I don't have a good run in front of me, but this should give you an idea.
>
> "True" unit tests, runs in 1-2 minutes like Jens Said:
> ./gradlew geode-core:test
>
> Single VM integration tests - runs in maybe 1.5 hours:
> ./gradlew geode-core:integrationTest
>
> Multiple VM integration tests - runs in maybe 5 hours?
> .gradlew geode-core:distributedTest
>
> Run all of the tests
> ./gradlew geode-core:precheckin
>
> -Dan
>
> On Tue, Jul 25, 2017 at 8:34 AM, Anthony Baker  wrote:
>
> > You might want to get a few thread dumps to see if there is a hung test.
> > Also, make sure you have sufficient RAM.
> >
> > Anthony
> >
> > > On Jul 25, 2017, at 1:13 AM, Dinesh Akhand 
> wrote:
> > >
> > > Hi Team,
> > >
> > > I trigger the geode-core Junit tests . it keeping running from 
> > > last
> > > 3
> > hour.
> > > Can you please let me know how much time it take completion of 
> > > Junit
> > test in Geode-core.
> > > I am using geode 1.1.1
> > >
> > >
> > > Thanks,
> > > Dinesh Akhand
> > > This message and the information contained herein is proprietary 
> > > and
> > confidential and subject to the Amdocs policy statement,
> > >
> > > you may review at https://www.amdocs.com/about/email-disclaimer <
> > https://www.amdocs.com/about/email-disclaimer>
> >
> >
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
>
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at

RE: continuous query internal mechanism

2017-08-08 Thread Roi Apelker 
Shukriya

What is the difference between registered events and CQ events?

-Original Message-
From: Anilkumar Gingade [mailto:aging...@pivotal.io] 
Sent: Monday, August 07, 2017 10:12 PM
To: dev@geode.apache.org
Subject: Re: continuous query internal mechanism

CQ Processing on server side is same for all clients (Java, C++)...

The subscription events are sent to client as ClientUpdateMessage, which holds 
information about registered events and CQ events. The client process this and 
updates/invokes the client side cache/listeners with respective event. Look 
into ClientUpdateMessageImpl and CacheClientUpdater (for client side 
processing).

-Anil.




On Mon, Aug 7, 2017 at 11:01 AM, Roi Apelker  wrote:

> Thanks,
>
> By the way, is there any difference in the behaviour of the server, if 
> the client that registered the CQ is a native (C++) client?
>
> I have been going over the classes and code for some time and can't 
> seem to find the actual location where a CQ update/notification is sent...
>
> It's like CqEventImpl class is never even generated in this scenario.
>
> If anyone can help here I would be most grateful :-)
>
> Thanks
>
> Roi
>
>
>
> -Original Message-
> From: Anilkumar Gingade [mailto:aging...@pivotal.io]
> Sent: Monday, August 07, 2017 8:23 PM
> To: dev@geode.apache.org
> Subject: Re: continuous query internal mechanism
>
> You can find those in CqServiceImpl.process*()...
>
> -Anil.
>
>
> On Mon, Aug 7, 2017 at 9:14 AM, Roi Apelker 
> wrote:
>
> > Hello,
> >
> > I am trying to look into the code of the continuous query mechanism 
> > - where the GEODE server sends the notification back to the client.
> >
> > Can anyone point me to the central classes of continuous query, 
> > especially to the one that is responsible for the calculation of the 
> > new data and packing it as a message back to the client?
> >
> > Thanks,
> >
> > Roi
> >
> > This message and the information contained herein is proprietary and 
> > confidential and subject to the Amdocs policy statement,
> >
> > you may review at https://www.amdocs.com/about/email-disclaimer < 
> > https://www.amdocs.com/about/email-disclaimer>
> >
> This message and the information contained herein is proprietary and 
> confidential and subject to the Amdocs policy statement,
>
> you may review at https://www.amdocs.com/about/email-disclaimer < 
> https://www.amdocs.com/about/email-disclaimer>
>
This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,

you may review at https://www.amdocs.com/about/email-disclaimer