[GitHub] incubator-geode issue #278: Feature/geode 1896 unable to specify a Partition...

[upthewaterspout]

Github user upthewaterspout commented on the issue:

https://github.com/apache/incubator-geode/pull/278
  
+1 Looks good.


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

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

[Jinmei Liao]

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

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 53566: GEODE-2078: Fix manifest classpath

[Mark Bretl]

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


Ship it!




Ship It!

- Mark Bretl


On Nov. 7, 2016, 8:23 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53566/
> ---
> 
> (Updated Nov. 7, 2016, 8:23 p.m.)
> 
> 
> Review request for geode, Mark Bretl and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The manifest classpath for *-dependencies.jar was pulling in
> dependencies from geode-pulse and geode-web-api. Filter out those
> projects before collecting the jars.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle cc2518b834fee59d8c7fc91233f2206c6a379c96 
> 
> Diff: https://reviews.apache.org/r/53566/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>

Re: Review Request 53566: GEODE-2078: Fix manifest classpath

[Dan Smith]

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


Ship it!




Ship It!

- Dan Smith


On Nov. 8, 2016, 4:23 a.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53566/
> ---
> 
> (Updated Nov. 8, 2016, 4:23 a.m.)
> 
> 
> Review request for geode, Mark Bretl and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The manifest classpath for *-dependencies.jar was pulling in
> dependencies from geode-pulse and geode-web-api. Filter out those
> projects before collecting the jars.
> 
> 
> Diffs
> -
> 
>   geode-assembly/build.gradle cc2518b834fee59d8c7fc91233f2206c6a379c96 
> 
> Diff: https://reviews.apache.org/r/53566/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>

Re: Review Request 53581: GEODE-2070 Improve documentation of region management

[Joey McAllister]

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


Ship it!




Ship It!

- Joey McAllister


On Nov. 8, 2016, 6:53 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53581/
> ---
> 
> (Updated Nov. 8, 2016, 6:53 p.m.)
> 
> 
> Review request for geode, Dave Barnes and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2070 Improve documentation of region management
> 
> 
> Diffs
> -
> 
>   geode-book/master_middleman/source/subnavs/geode-subnav.erb 
> 0f1292c23bfe6222c808f84f19e3ac78cf69a635 
>   geode-docs/basic_config/data_regions/chapter_overview.html.md.erb 
> 4917259e06345a8e6fc9ad56fc8045961e151fd5 
>   geode-docs/basic_config/data_regions/create_a_region_with_API.html.md.erb 
> 8bb359b86a0eaeb88fa6fef8432bef89aa872a8b 
>   
> geode-docs/basic_config/data_regions/create_a_region_with_cacheXML.html.md.erb
>  d7838f64f4ed462c26a801911bfe7948891760e0 
>   geode-docs/basic_config/data_regions/managing_data_regions.html.md.erb 
> f4f792eae3a60efaa52d782f9fc0751656ccf58f 
>   geode-docs/reference/topics/cache_xml.html.md.erb 
> 8d3187b49eff8197f153d6832dc522ce503b1027 
>   geode-docs/reference/topics/chapter_overview_regionshortcuts.html.md.erb 
> 85c69e7c52b03b93efd8ce7be6be698935560fac 
>   geode-docs/reference/topics/client-cache.html.md.erb 
> 963f57434e488c89749163408848b27f714beabd 
>   geode-docs/reference/topics/gfe_cache_xml.html.md.erb 
> 2cd78ec933277fc2be05bb3021bced32a5034192 
> 
> Diff: https://reviews.apache.org/r/53581/diff/
> 
> 
> Testing
> ---
> 
> gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

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

[Jinmei Liao]

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

(Updated Nov. 8, 2016, 8:53 p.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 (updated)
-

  
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-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 cce4fdbb2e3a35fd0727b74baf07132d77a324dd 

Diff: https://reviews.apache.org/r/53579/diff/


Testing (updated)
---

precheckin successful


Thanks,

Jinmei Liao

Re: Review Request 53581: GEODE-2070 Improve documentation of region management

[Dave Barnes]

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


Ship it!




Ship It!

- Dave Barnes


On Nov. 8, 2016, 6:53 p.m., Karen Miller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53581/
> ---
> 
> (Updated Nov. 8, 2016, 6:53 p.m.)
> 
> 
> Review request for geode, Dave Barnes and Joey McAllister.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2070 Improve documentation of region management
> 
> 
> Diffs
> -
> 
>   geode-book/master_middleman/source/subnavs/geode-subnav.erb 
> 0f1292c23bfe6222c808f84f19e3ac78cf69a635 
>   geode-docs/basic_config/data_regions/chapter_overview.html.md.erb 
> 4917259e06345a8e6fc9ad56fc8045961e151fd5 
>   geode-docs/basic_config/data_regions/create_a_region_with_API.html.md.erb 
> 8bb359b86a0eaeb88fa6fef8432bef89aa872a8b 
>   
> geode-docs/basic_config/data_regions/create_a_region_with_cacheXML.html.md.erb
>  d7838f64f4ed462c26a801911bfe7948891760e0 
>   geode-docs/basic_config/data_regions/managing_data_regions.html.md.erb 
> f4f792eae3a60efaa52d782f9fc0751656ccf58f 
>   geode-docs/reference/topics/cache_xml.html.md.erb 
> 8d3187b49eff8197f153d6832dc522ce503b1027 
>   geode-docs/reference/topics/chapter_overview_regionshortcuts.html.md.erb 
> 85c69e7c52b03b93efd8ce7be6be698935560fac 
>   geode-docs/reference/topics/client-cache.html.md.erb 
> 963f57434e488c89749163408848b27f714beabd 
>   geode-docs/reference/topics/gfe_cache_xml.html.md.erb 
> 2cd78ec933277fc2be05bb3021bced32a5034192 
> 
> Diff: https://reviews.apache.org/r/53581/diff/
> 
> 
> Testing
> ---
> 
> gradle rat check passes
> 
> 
> Thanks,
> 
> Karen Miller
> 
>

Review Request 53581: GEODE-2070 Improve documentation of region management

[Karen Miller]

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

Review request for geode, Dave Barnes and Joey McAllister.


Repository: geode


Description
---

GEODE-2070 Improve documentation of region management


Diffs
-

  geode-book/master_middleman/source/subnavs/geode-subnav.erb 
0f1292c23bfe6222c808f84f19e3ac78cf69a635 
  geode-docs/basic_config/data_regions/chapter_overview.html.md.erb 
4917259e06345a8e6fc9ad56fc8045961e151fd5 
  geode-docs/basic_config/data_regions/create_a_region_with_API.html.md.erb 
8bb359b86a0eaeb88fa6fef8432bef89aa872a8b 
  
geode-docs/basic_config/data_regions/create_a_region_with_cacheXML.html.md.erb 
d7838f64f4ed462c26a801911bfe7948891760e0 
  geode-docs/basic_config/data_regions/managing_data_regions.html.md.erb 
f4f792eae3a60efaa52d782f9fc0751656ccf58f 
  geode-docs/reference/topics/cache_xml.html.md.erb 
8d3187b49eff8197f153d6832dc522ce503b1027 
  geode-docs/reference/topics/chapter_overview_regionshortcuts.html.md.erb 
85c69e7c52b03b93efd8ce7be6be698935560fac 
  geode-docs/reference/topics/client-cache.html.md.erb 
963f57434e488c89749163408848b27f714beabd 
  geode-docs/reference/topics/gfe_cache_xml.html.md.erb 
2cd78ec933277fc2be05bb3021bced32a5034192 

Diff: https://reviews.apache.org/r/53581/diff/


Testing
---

gradle rat check passes


Thanks,

Karen Miller

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

[Jinmei Liao]

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


Ship it!




Ship It!

- Jinmei Liao


On Nov. 8, 2016, 5:54 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53577/
> ---
> 
> (Updated Nov. 8, 2016, 5:54 p.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 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
> 
>

[GitHub] incubator-geode issue #280: GEODE-1740: Potential region inconsistency with ...

[gemzdude]

Github user gemzdude commented on the issue:

https://github.com/apache/incubator-geode/pull/280
  
Closed as incomplete


---
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] incubator-geode pull request #280: GEODE-1740: Potential region inconsistenc...

[gemzdude]

Github user gemzdude closed the pull request at:

https://github.com/apache/incubator-geode/pull/280


---
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 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 53579: GEODE-1570: add a test to verify rest security with SSL.

[Jinmei Liao]

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

(Updated Nov. 8, 2016, 6:13 p.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 (updated)
---

precheckin


Thanks,

Jinmei Liao

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

[Jinmei Liao]

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

Review request for geode.


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


Thanks,

Jinmei Liao

Review Request 53577: Remove illegal javadoc tag that generates warning

[Kirk Lund]

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

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

[GitHub] incubator-geode issue #276: GEODE-1993: postprocess region/key

[kjduling]

Github user kjduling commented on the issue:

https://github.com/apache/incubator-geode/pull/276
  
1. I am using Autowire, the annotation is on the base class's constructor.  
But as for the injection via the constructor, please review 
https://spring.io/blog/2015/11/29/how-not-to-hate-spring-in-2016 which 
references http://olivergierke.de/2013/11/why-field-injection-is-evil/ and 
http://docs.spring.io/spring-framework/docs/current/spring-framework-reference/htmlsingle/#beans-constructor-injection

Do you think it would be better to only add the RestSecurityService to the 
controllers that actually need it instead of adding it to the base class?  I 
think that would complicate the constructors.

2 & 3. Agreed, that'll make the test cleaner.


---
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 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

[Bruce Schuchardt]

> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3323
> > 
> >
> > Is ".equals" too specific? You want something like instanceof. For 
> > example if the value-constraint was "Number" you would want it to be 
> > satisfied if "valueClassName" was "Integer", "Long", etc.
> > 
> > You could do this with Class.isAssignableFrom(this.valueConstraint) but 
> > in that case you need to load a class for "valueClassName". One of the 
> > goals of pdx is that you do not need to be able to load the domain classes 
> > on the server, so you might not want to do this check. If you leave it with 
> > just a simple equals check you should document that in the case of pdx the 
> > value-constraint needs to exactly match (i.e. no instanceof support).
> 
> Bruce Schuchardt wrote:
> In the case of Rest objects there are no classes to perform this kind of 
> check so equals() is the correct operation to use.  For non-Pdx objects we 
> already use instanceof checks.  For non-Pdx objects we perform no constraint 
> check at all because it's already been performed and we can't be assured that 
> we have classes for the objects held by the PdxInstance anyway.

Edited: For non-Pdx objects we already use instanceof checks.  For 
**PdxInstance objects** we perform no constraint check at all because it's 
already been performed and we can't be assured that we have classes for the 
objects held by the PdxInstance anyway.


- Bruce


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


On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53557/
> ---
> 
> (Updated Nov. 7, 2016, 10:13 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2080
> https://issues.apache.org/jira/browse/GEODE-2080
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If you set a value constraint on a cache Region you will be unable to store 
> objects in the region via the Rest API.  This change-set modifies 
> LocalRegion's constraint check to look for a Rest document and use its type 
> name in the constraint check
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 
> 
> Diff: https://reviews.apache.org/r/53557/diff/
> 
> 
> Testing
> ---
> 
> new test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Review Request 53557: GEODE-2080 Rest POST put call not working with region valueConstrain

[Bruce Schuchardt]

> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3315
> > 
> >
> > If pdx.getClassName() is not equals to JSON_CLASSNAME then would it 
> > make sense to set valueClassName to pdx.getClassName()? That way you could 
> > have "read-serialized" set to true and still have a meaningful value 
> > constraint with pdx

No, definitely not.  That would cause it to start rejecting PdxInstances of 
subclasses of the constraint class.

For example, if the region is constrained to hold Person objects and a client 
entered an Employee object (subclass of Person) into the region a server would 
reject the value if we checked that the PdxInstance's class name had to match 
Person.

It's enough that the original cache that entered the object into the system 
performed a constraint check on the original object before it was serialized.  
That's not the case for Json objects coming from the Rest API - they need to be 
checked and that is the reason for this change.


> On Nov. 8, 2016, 1:44 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java, 
> > line 3323
> > 
> >
> > Is ".equals" too specific? You want something like instanceof. For 
> > example if the value-constraint was "Number" you would want it to be 
> > satisfied if "valueClassName" was "Integer", "Long", etc.
> > 
> > You could do this with Class.isAssignableFrom(this.valueConstraint) but 
> > in that case you need to load a class for "valueClassName". One of the 
> > goals of pdx is that you do not need to be able to load the domain classes 
> > on the server, so you might not want to do this check. If you leave it with 
> > just a simple equals check you should document that in the case of pdx the 
> > value-constraint needs to exactly match (i.e. no instanceof support).

In the case of Rest objects there are no classes to perform this kind of check 
so equals() is the correct operation to use.  For non-Pdx objects we already 
use instanceof checks.  For non-Pdx objects we perform no constraint check at 
all because it's already been performed and we can't be assured that we have 
classes for the objects held by the PdxInstance anyway.


- Bruce


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


On Nov. 7, 2016, 10:13 p.m., Bruce Schuchardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53557/
> ---
> 
> (Updated Nov. 7, 2016, 10:13 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, Hitesh Khamesra, and Udo Kohlmeyer.
> 
> 
> Bugs: GEODE-2080
> https://issues.apache.org/jira/browse/GEODE-2080
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> If you set a value constraint on a cache Region you will be unable to store 
> objects in the region via the Rest API.  This change-set modifies 
> LocalRegion's constraint check to look for a Rest document and use its type 
> name in the constraint check
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> 3873e6e159ebba4c1a288e9fccde5dbabd2a1140 
>   geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
> 9a9680a7e14ddec91f005fa0f0c6c3da8d033df2 
> 
> Diff: https://reviews.apache.org/r/53557/diff/
> 
> 
> Testing
> ---
> 
> new test, precheckin
> 
> 
> Thanks,
> 
> Bruce Schuchardt
> 
>

Re: Build failed in Jenkins: Geode-nightly #647

[Nabarun Nag]

spotlessApply added to DiskSpaceLimitIntegrationTest.java and pushed to
develop.

- local build was successful with no error.

On Tue, Nov 8, 2016 at 8:06 AM Nabarun Nag  wrote:

> Fixing it.
> On Tue, Nov 8, 2016 at 7:48 AM Apache Jenkins Server <
> jenk...@builds.apache.org> wrote:
>
> See 
>
> Changes:
>
> [upthewaterspout] Adding a geode-benchmark project with support for
> running jmh benchmarks
>
> [upthewaterspout] GEODE-1985 Removing some string comparisons in the
> AttributesDescriptor
>
> [upthewaterspout] GEODE-1985: Updating the SAFE_QUERY_TIME after updating
> indexes
>
> [jiliao] GEODE-2079: mark the test as flaky
>
> [ukohlmeyer] GEODE-2017: removal of nonPRSingleHop stat in client
>
> [ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for
> every p2p
>
> [ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for
> every p2p
>
> [ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for
> every p2p
>
> [ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for
> every p2p
>
> [ukohlmeyer] GEODE-1874: Checkin after code formatting refactor
>
> [ukohlmeyer] GEODE-1874: Fix formatting
>
> [Anil] GEODE-2064: Added check for system shutdown while handlling connect
>
> [nnag] GEODE-2063: Increased the timeout on async thread join
>
> [dschneider] GEODE-1971: fix shutDownAll hang
>
> [eshu] GEODE-2077: Throw appropriate exceptions when get op in a
> transaction
>
> [klund] GEODE-2082: fix flakiness in sameKeepsOneFile
>
> [klund] GEODE-2082: rename aboveZeroKeepsAtLeastOneFile to
>
> --
> [...truncated 519 lines...]
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-cq:processTestResources
> :geode-cq:testClasses
> :geode-cq:checkMissedTests
> :geode-cq:spotlessJavaCheck
> :geode-cq:spotlessCheck
> :geode-cq:test
> :geode-cq:check
> :geode-cq:build
> :geode-cq:distributedTest
> :geode-cq:flakyTest
> :geode-cq:integrationTest
> :geode-json:assemble
> :geode-json:compileTestJava UP-TO-DATE
> :geode-json:processTestResources UP-TO-DATE
> :geode-json:testClasses UP-TO-DATE
> :geode-json:checkMissedTests UP-TO-DATE
> :geode-json:spotlessJavaCheck
> :geode-json:spotlessCheck
> :geode-json:test UP-TO-DATE
> :geode-json:check
> :geode-json:build
> :geode-json:distributedTest UP-TO-DATE
> :geode-json:flakyTest UP-TO-DATE
> :geode-json:integrationTest UP-TO-DATE
> :geode-junit:javadoc
> :geode-junit:javadocJar
> :geode-junit:sourcesJar
> :geode-junit:signArchives SKIPPED
> :geode-junit:assemble
> :geode-junit:compileTestJava
> :geode-junit:processTestResources UP-TO-DATE
> :geode-junit:testClasses
> :geode-junit:checkMissedTests
> :geode-junit:spotlessJavaCheck
> :geode-junit:spotlessCheck
> :geode-junit:test
> :geode-junit:check
> :geode-junit:build
> :geode-junit:distributedTest
> :geode-junit:flakyTest
> :geode-junit:integrationTest
> :geode-lucene:assemble
> :geode-lucene:compileTestJavaNote: Some input files use or override a
> deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-lucene:processTestResources
> :geode-lucene:testClasses
> :geode-lucene:checkMissedTests
> :geode-lucene:spotlessJavaCheck
> :geode-lucene:spotlessCheck
> :geode-lucene:test
> :geode-lucene:check
> :geode-lucene:build
> :geode-lucene:distributedTest
> :geode-lucene:flakyTest
> :geode-lucene:integrationTest
> :geode-old-client-support:assemble
> :geode-old-client-support:compileTestJava
> :geode-old-client-support:processTestResources UP-TO-DATE
> :geode-old-client-support:testClasses
> :geode-old-client-support:checkMissedTests
> :geode-old-client-support:spotlessJavaCheck
> :geode-old-client-support:spotlessCheck
> :geode-old-client-support:test
> :geode-old-client-support:check
> :geode-old-client-support:build
> :geode-old-client-support:distributedTest
> :geode-old-client-support:flakyTest
> :geode-old-client-support:integrationTest
> :geode-pulse:assemble
> :geode-pulse:compileTestJavaNote: <
> https://builds.apache.org/job/Geode-nightly/ws/geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/ui/PulseAbstractTest.java>
> uses or overrides a deprecated API.
> Note: Recompile with -Xlint:deprecation for details.
> Note: Some input files use unchecked or unsafe operations.
> Note: Recompile with -Xlint:unchecked for details.
>
> :geode-pulse:processTestResources
> :geode-pulse:testClasses
> :geode-pulse:checkMissedTests
> :geode-pulse:spotlessJavaCheck
> :geode-pulse:spotlessCheck
> :geode-pulse:test
> :geode-pulse:check
> :geode-pulse:build
> :geode-pulse:distributedTest
> :geode-pulse:flakyTest
> :geode-pulse:integrationTest
> :geode-rebalancer:assemble
> :geode-rebalancer:compileTestJava
> :geode-rebalancer:processTestResources UP-TO-DATE
> :geode-rebalancer:testClasses
> 

Build failed in Jenkins: Geode-nightly #647

[Apache Jenkins Server]

See 

Changes:

[upthewaterspout] Adding a geode-benchmark project with support for running jmh 
benchmarks

[upthewaterspout] GEODE-1985 Removing some string comparisons in the 
AttributesDescriptor

[upthewaterspout] GEODE-1985: Updating the SAFE_QUERY_TIME after updating 
indexes

[jiliao] GEODE-2079: mark the test as flaky

[ukohlmeyer] GEODE-2017: removal of nonPRSingleHop stat in client

[ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for 
every p2p

[ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for 
every p2p

[ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for 
every p2p

[ukohlmeyer] GEODE-1874: Changed setNextNeighbor to not create HashMap for 
every p2p

[ukohlmeyer] GEODE-1874: Checkin after code formatting refactor

[ukohlmeyer] GEODE-1874: Fix formatting

[Anil] GEODE-2064: Added check for system shutdown while handlling connect

[nnag] GEODE-2063: Increased the timeout on async thread join

[dschneider] GEODE-1971: fix shutDownAll hang

[eshu] GEODE-2077: Throw appropriate exceptions when get op in a transaction

[klund] GEODE-2082: fix flakiness in sameKeepsOneFile

[klund] GEODE-2082: rename aboveZeroKeepsAtLeastOneFile to

--
[...truncated 519 lines...]
Note: Recompile with -Xlint:unchecked for details.

:geode-cq:processTestResources
:geode-cq:testClasses
:geode-cq:checkMissedTests
:geode-cq:spotlessJavaCheck
:geode-cq:spotlessCheck
:geode-cq:test
:geode-cq:check
:geode-cq:build
:geode-cq:distributedTest
:geode-cq:flakyTest
:geode-cq:integrationTest
:geode-json:assemble
:geode-json:compileTestJava UP-TO-DATE
:geode-json:processTestResources UP-TO-DATE
:geode-json:testClasses UP-TO-DATE
:geode-json:checkMissedTests UP-TO-DATE
:geode-json:spotlessJavaCheck
:geode-json:spotlessCheck
:geode-json:test UP-TO-DATE
:geode-json:check
:geode-json:build
:geode-json:distributedTest UP-TO-DATE
:geode-json:flakyTest UP-TO-DATE
:geode-json:integrationTest UP-TO-DATE
:geode-junit:javadoc
:geode-junit:javadocJar
:geode-junit:sourcesJar
:geode-junit:signArchives SKIPPED
:geode-junit:assemble
:geode-junit:compileTestJava
:geode-junit:processTestResources UP-TO-DATE
:geode-junit:testClasses
:geode-junit:checkMissedTests
:geode-junit:spotlessJavaCheck
:geode-junit:spotlessCheck
:geode-junit:test
:geode-junit:check
:geode-junit:build
:geode-junit:distributedTest
:geode-junit:flakyTest
:geode-junit:integrationTest
:geode-lucene:assemble
:geode-lucene:compileTestJavaNote: Some input files use or override a 
deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-lucene:processTestResources
:geode-lucene:testClasses
:geode-lucene:checkMissedTests
:geode-lucene:spotlessJavaCheck
:geode-lucene:spotlessCheck
:geode-lucene:test
:geode-lucene:check
:geode-lucene:build
:geode-lucene:distributedTest
:geode-lucene:flakyTest
:geode-lucene:integrationTest
:geode-old-client-support:assemble
:geode-old-client-support:compileTestJava
:geode-old-client-support:processTestResources UP-TO-DATE
:geode-old-client-support:testClasses
:geode-old-client-support:checkMissedTests
:geode-old-client-support:spotlessJavaCheck
:geode-old-client-support:spotlessCheck
:geode-old-client-support:test
:geode-old-client-support:check
:geode-old-client-support:build
:geode-old-client-support:distributedTest
:geode-old-client-support:flakyTest
:geode-old-client-support:integrationTest
:geode-pulse:assemble
:geode-pulse:compileTestJavaNote: 

 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-pulse:processTestResources
:geode-pulse:testClasses
:geode-pulse:checkMissedTests
:geode-pulse:spotlessJavaCheck
:geode-pulse:spotlessCheck
:geode-pulse:test
:geode-pulse:check
:geode-pulse:build
:geode-pulse:distributedTest
:geode-pulse:flakyTest
:geode-pulse:integrationTest
:geode-rebalancer:assemble
:geode-rebalancer:compileTestJava
:geode-rebalancer:processTestResources UP-TO-DATE
:geode-rebalancer:testClasses
:geode-rebalancer:checkMissedTests
:geode-rebalancer:spotlessJavaCheck
:geode-rebalancer:spotlessCheck
:geode-rebalancer:test
:geode-rebalancer:check
:geode-rebalancer:build
:geode-rebalancer:distributedTest
:geode-rebalancer:flakyTest
:geode-rebalancer:integrationTest
:geode-wan:assemble
:geode-wan:compileTestJavaNote: Some input files use or override a deprecated 
API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:geode-wan:processTestResources

RE: classpath in geode release

[Gal Palmery]

Gr8, now all is clear :)
Thanks!
Gal

-Original Message-
From: Anthony Baker [mailto:aba...@pivotal.io] 
Sent: Tuesday, November 08, 2016 16:17
To: dev@geode.incubator.apache.org
Subject: Re: classpath in geode release


> On Nov 8, 2016, at 1:09 AM, Gal Palmery  wrote:
> 
> Actually I do see them in the lib dir, after running the build, inside the 
> zip in the distributions:
>   
> geode-assembly\build\distributions\apache-geode-1.0.0-incubating.zip\a
> pache-geode-1.0.0-incubating\lib also, when comparing to the Gemfire 
> distribution (that we use in our product in released versions) - the 
> gfsh_dependencies.jar is present in the lib dir as well.

You’re correct.  What I meant was that the extra jars referenced in the 
manifest classpath (jackson-annotations-2.8.2.jar for example) are not present 
in the lib/ directory and thus won’t be present on the classpath in practice.

> If we start gemfire/geode from gfsh, are these jars used for creating the 
> gemfire/geode runtime classpath?

Yes


Anthony


This message and the information contained herein is proprietary and 
confidential and subject to the Amdocs policy statement,
you may review at http://www.amdocs.com/email_disclaimer.asp

Re: classpath in geode release

[Anthony Baker]

> On Nov 8, 2016, at 1:09 AM, Gal Palmery  wrote:
> 
> Actually I do see them in the lib dir, after running the build, inside the 
> zip in the distributions:
>   
> geode-assembly\build\distributions\apache-geode-1.0.0-incubating.zip\apache-geode-1.0.0-incubating\lib
> also, when comparing to the Gemfire distribution (that we use in our product 
> in released versions) - the gfsh_dependencies.jar is present in the lib dir 
> as well. 

You’re correct.  What I meant was that the extra jars referenced in the 
manifest classpath (jackson-annotations-2.8.2.jar for example) are not present 
in the lib/ directory and thus won’t be present on the classpath in practice.

> If we start gemfire/geode from gfsh, are these jars used for creating the 
> gemfire/geode runtime classpath?

Yes


Anthony

RE: classpath in geode release

[Gal Palmery]

Actually I do see them in the lib dir, after running the build, inside the zip 
in the distributions:

geode-assembly\build\distributions\apache-geode-1.0.0-incubating.zip\apache-geode-1.0.0-incubating\lib
also, when comparing to the Gemfire distribution (that we use in our product in 
released versions) - the gfsh_dependencies.jar is present in the lib dir as 
well. 
If we start gemfire/geode from gfsh, are these jars used for creating the 
gemfire/geode runtime classpath?

Gal


-Original Message-
From: Anthony Baker [mailto:aba...@pivotal.io] 
Sent: Monday, November 07, 2016 18:40
To: dev@geode.incubator.apache.org
Subject: Re: classpath in geode release


> On Nov 7, 2016, at 1:11 AM, Gal Palmery  wrote:
> 
> Thanks Anthony.
> So, if I were to manually remove them from the manifest, do you expect any 
> issues at runtime?
> 

Since those jars are not present in the lib/ dir, the MANFEST classpath entry 
should be harmless either way.

> Also, you mentioned that these are only used when using cmd line. How is the 
> runtime classpath built when not using cmd ?
> 

The dependency jars are only used when launching the server.  Applications 
using the gemfire java client can use maven / gradle dependencies to pull in 
the correct jars.

> Thanks,
> Gal
> 
> 
> -Original Message-
> From: Anthony Baker [mailto:aba...@pivotal.io] 
> Sent: Monday, November 07, 2016 01:20
> To: geode
> Subject: Re: classpath in geode release
> 
> 
>> On Nov 6, 2016, at 7:47 AM, Gal Palmery  wrote:
>> 
>> Clarification - 
>> This is in the geode-dependencies.jar and gfsh-dependencies.jar that were 
>> created by the local build that I run on my PC. 
>> I downloaded the 1.0.0-incubating release and built it using the following 
>> command:
>>> gradlew clean build installDist -Dskip.tests=true
>> 
> 
> Yep, this looks like a bug, though probably harmless.  The geode-core 
> dependencies pull in v2.8.0 transitively:
> 
> +--- com.fasterxml.jackson.core:jackson-databind:2.8.2
> |+--- com.fasterxml.jackson.core:jackson-annotations:2.8.0
> |\--- com.fasterxml.jackson.core:jackson-core:2.8.2
> 
> while the geode-pulse and geode-web-api modules pull in v2.8.2 with an 
> explicit dependency.  These should be filtered when we build the classpath.  
> See GEODE-2078.
> 
>> where can I find these jars (geode-dependencies.jar and 
>> gfsh-dependencies.jar) that are created each nightly build? 
>> I tried in the maven central repository, but they are not there.
> 
> There jars aren’t published to maven as they are manifest-only jars meant for 
> cmd line usage.  The build artifacts can be found in jenkins (see 
> https://builds.apache.org/job/Geode-nightly) but those should only be used 
> for development purposes and are not official releases.
> 
>> 
>> Thanks,
>> Gal
>> 
>> 
>> -Original Message-
>> From: Gal Palmery 
>> Sent: Sunday, November 06, 2016 11:43
>> To: dev@geode.incubator.apache.org
>> Subject: classpath in geode release
>> 
>> Hi,
>> 
>> I noticed that there are 2 jackson-annotations jars, and 2 slf4j-api jars 
>> with different versions in the classpath (in the MANIFESTs of 
>> geode-dependencies.jar and in gfsh-dependencies.jar).
>> What is the reason for that? Is this a bug?
>> 
>> Thanks,
>> Gal
>> 
>> This message and the information contained herein is proprietary and 
>> confidential and subject to the Amdocs policy statement, you may review at 
>> http://www.amdocs.com/email_disclaimer.asp
>