Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-25 Thread Anthony Baker
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50436/ --- Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Kirk Lund
John, I'll save my questions for your SDG talk. Thanks! -Kirk On Mon, Jul 25, 2016 at 6:59 PM, John Blum wrote: > For *Spring (Data GemFire)* applications, I don't strictly refuse this > configuration, as it is a valid configuration. It's just *not* enabled by > *default*

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread John Blum
For *Spring (Data GemFire)* applications, I don't strictly refuse this configuration, as it is a valid configuration. It's just *not* enabled by *default* since most *Spring* configured members (GemFire peer cache) are "applications" that use the GemFire components/objects (Cache, Regions, etc)

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Kirk Lund
It sounds like maybe we need to modify the docs to tell users to not enable reconnect when configuring the Cache with Spring. Is there anything else we can do like detect this in some way and refuse to allow the configuration to startup? -Kirk On Monday, July 25, 2016, Bruce Schuchardt

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Kirk Lund
I don't believe there's a problem with Shiro using ThreadLocals. My original intention of this discussion thread was to promote Unit Testing and use of Mockito for any feature that's more complex than StringUtils.isEmpty(String value). Just because the security class is currently called

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Kirk Lund
I'm specifically advocating wrapping the class with an interface to facilitate mocking for unit testing classes that depend on it. This way we can use Mockito ArgumentCaptor to assert that the correct permission strings are being passed to our security component. We can also provide fake returns

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Jacob Barrett
Security is one of those exceptions to the rules about ThreadLocal. Almost every implementation uses ThreadLocal to stash the current executing context. Generally then there is a static class that gets the current context. To test you should be able to just push your own mocked context into the

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Jinmei Liao
Shiro relies heavily on ThreadLocal. GeodeSecurityUtils is a wrapper around Shiro. That's where all the static usage stems from. There is a trade-off if we pass some sort session object around: calling code would need to keep a reference of it and knows when/where/which to pass it down. On Mon,

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Dan Smith
IMHO static helper methods like those in Bytes are fine. We would never want to substitute a mock implementation of one of those methods and they don't have any collaborators. Just glancing at GeodeSecurityUtils, what I don't like is that it's relying heavily on mutable static singletons

[GitHub] incubator-geode pull request #217: feature/GEODE-11-Defining lucene index

2016-07-25 Thread aparnard
GitHub user aparnard opened a pull request: https://github.com/apache/incubator-geode/pull/217 feature/GEODE-11-Defining lucene index - Added a definedIndexMap field in LuceneServiceImpl to store the uninitialized lucene indexes - Added a field "status" to the list and

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

2016-07-25 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50141/#review143454 --- Ship it! Ship It! - Kirk Lund On July 18, 2016, 3:57 p.m.,

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

2016-07-25 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50051/#review143453 --- Ship it! Ship It! - Kirk Lund On July 14, 2016, 10:19 p.m.,

Re: Review Request 50245: GEODE-1647: Add Integrated Security to Peer Authentication

2016-07-25 Thread Kirk Lund
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50245/#review143452 --- -1 There is considerable refactoring and use of Mockito missing

[GitHub] incubator-geode issue #216: Feature/geode 1694

2016-07-25 Thread davebarnes97
Github user davebarnes97 commented on the issue: https://github.com/apache/incubator-geode/pull/216 +1 --- 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

[GitHub] incubator-geode pull request #216: Feature/geode 1694

2016-07-25 Thread joeymcallister
GitHub user joeymcallister opened a pull request: https://github.com/apache/incubator-geode/pull/216 Feature/geode 1694 GEODE-1694 Add Karen Smoler Miller to commiters You can merge this pull request into a Git repository by running: $ git pull

Re: Unit testing classes that depend on static collaborators

2016-07-25 Thread Kirk Lund
I was trying to describe any Java class in Geode which provides feature functionality that should be unit tested and integration tested, but is either currently implemented as a static class or which collaborates with a non-trivial component currently implemented as a static class. I wasn't

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

2016-07-25 Thread Spring CI
--- Spring Data GemFire > Nightly-ApacheGeode > #381 was successful. --- Scheduled 1423 tests in total. https://build.spring.io/browse/SGF-NAG-381/ -- This

Re: Review Request 50420: there is a race that cache is still initializing, some components are not ready

2016-07-25 Thread xiaojian zhou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50420/ --- (Updated July 25, 2016, 10:18 p.m.) Review request for geode and Dan Smith.

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Bruce Schuchardt
But as John pointed out, you shouldn't use auto-reconnect with Spring. Spring has injected references to the old cache and regions into applications and they won't know about the new cache. Auto-reconnect doesn't rebuild the old cache - it creates a new one with its own identity. Le

[GitHub] incubator-geode pull request #:

2016-07-25 Thread kjduling
Github user kjduling commented on the pull request: https://github.com/apache/incubator-geode/commit/f0d36eea5da1102d7293fd2fd573c7ad3ae01d66#commitcomment-18387220 +1 - oversight on my part. --- If your project is set up for it, you can reply to this email and have your reply

Re: Review Request 50420: there is a race that cache is still initializing, some components are not ready

2016-07-25 Thread Jason Huynh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50420/#review143438 ---

Re: Review Request 50419: GEODE-1617: Metadata region was not having a cache listener attached

2016-07-25 Thread William Markito
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50419/#review143437 --- Ship it! Ship It! - William Markito On July 25, 2016, 9:38

Re: Review Request 50420: there is a race that cache is still initializing, some components are not ready

2016-07-25 Thread Dan Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50420/#review143436 --- Is this supposed to wait until the queue is not null? If so, I

Review Request 50420: there is a race that cache is still initializing, some components are not ready

2016-07-25 Thread xiaojian zhou
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50420/ --- Review request for geode and Dan Smith. Bugs: GEODE-1671

Review Request 50419: GEODE-1617: Metadata region was not having a cache listener attached

2016-07-25 Thread Jason Huynh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50419/ --- Review request for geode, Kevin Duling and William Markito. Repository: geode

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Barry Oglesby
I see the difference in our tests. Juan is using spring to configure the cache, not cache.xml. In my test, I used just cache.xml. I read in his bug "through spring or cache.xml", so I just did cache.xml, but its the spring part of that statement that is relevant. With spring config, the cache is

Re: Proposal - Fail cache creation on persistent PR colocation misconfiguration

2016-07-25 Thread John Blum
Ah, I see, thanks Dan. On Mon, Jul 25, 2016 at 1:03 PM, Dan Smith wrote: > On Mon, Jul 25, 2016 at 12:19 PM, John Blum wrote: > > > *> The issue is that even though you have everything defined in > cache.xml, > > if you have manual-start set to true on the

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Kirk Lund
I wouldn't recommend using both together as this combo is completely untested. Users who do this are literally the only folks who have even tried running this way. I recommend supporting only what we've tested. -Kirk On Mon, Jul 25, 2016 at 12:27 PM, Michael Stolz wrote: >

Re: Proposal - Fail cache creation on persistent PR colocation misconfiguration

2016-07-25 Thread Dan Smith
On Mon, Jul 25, 2016 at 12:19 PM, John Blum wrote: > *> The issue is that even though you have everything defined in cache.xml, > if you have manual-start set to true on the gateway it will not create the > colocated regions until you call start* > > Why is that? I am not sure

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Michael Stolz
There are still some things that can't be configured with cluster configuration service so the combination of cluster-configuration-service=true and cache.xml will have to be supported until such time as CCS is completed. -- Mike Stolz Principal Engineer, GemFire Product Manager Mobile:

Re: Proposal - Fail cache creation on persistent PR colocation misconfiguration

2016-07-25 Thread John Blum
*> The issue is that even though you have everything defined in cache.xml, if you have manual-start set to true on the gateway it will not create the colocated regions until you call start* Why is that? I am not sure I understand why collocated (child) *Regions* would not be (or prevented from

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread John Blum
Hi Kirk- FYI, SDG disables [1] *cluster config*, and especially, *auto-reconnect* by default. Having a peer cache "application" configured

Re: use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Barry Oglesby
I'm not seeing this behavior in my tests. I have a cache.xml and use-cluster-configuration=true. When the member recovers, I see the xml being recreated. The GMSMembershipManager.saveCacheXmlForReconnect method is being invoked but short-circuiting because sharedConfigEnabled=true. But, the

Update on Apache Geode M3 release

2016-07-25 Thread William Markito
Folks, I was waiting for the stabilization of release branch and considering options after looking at the errors, but as it currently stand, given that we're having successful builds on develop, I'll be restarting the release process branching off develop once again for M3 and RC4 should be out

Re: Review Request 50364: Adding support for Tomcat 8.5.4 for the session module

2016-07-25 Thread Jason Huynh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50364/ --- (Updated July 25, 2016, 6:32 p.m.) Review request for geode, anilkumar

Re: Review Request 50399: GEODE-1692: fix IllegalStateException in Transaction when the node is disconnecting from the ds

2016-07-25 Thread Darrel Schneider
> On July 25, 2016, 11 a.m., Darrel Schneider wrote: > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXLockRequest.java, > > line 99 > > > > > > I think it is wrong for this release method to call

Re: Review Request 50399: GEODE-1692: fix IllegalStateException in Transaction when the node is disconnecting from the ds

2016-07-25 Thread Darrel Schneider
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50399/#review143405 --- Ship it! Ship It! - Darrel Schneider On July 25, 2016,

[GitHub] incubator-geode pull request #212: GEODE-1673: fail start of locator or serv...

2016-07-25 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/212 --- 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

Re: Proposal - Fail cache creation on persistent PR colocation misconfiguration

2016-07-25 Thread Dan Smith
I'm generally in favor of this idea. One place you might get in trouble with adding this check is that we have an outstanding bug with gateway sender manual start - GEODE-1117. The issue is that even though you have everything defined in cache.xml, if you have manual-start set to true on the

Re: Review Request 50399: GEODE-1692: fix IllegalStateException in Transaction when the node is disconnecting from the ds

2016-07-25 Thread Darrel Schneider
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50399/#review143402 ---

use-cluster-configuration/cache.xml/reconnect

2016-07-25 Thread Kirk Lund
Below came to me via support engineer, and I'm trying to determine what the correct behavior should be: "Let's assume that you're configuring your entire cache through spring or cache.xml without using the cluster configuration service, and that you're using the default settings when starting the

Review Request 50399: GEODE-1692: fix IllegalStateException in Transaction when the node is disconnecting from the ds

2016-07-25 Thread Eric Shu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50399/ --- Review request for geode, Darrel Schneider and Swapnil Bawaskar. Repository:

[GitHub] incubator-geode pull request #201: GEODE-1617: Regions can be created with a...

2016-07-25 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-geode/pull/201 --- 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

Re: Proposal - Fail cache creation on persistent PR colocation misconfiguration

2016-07-25 Thread Michael Stolz
What about the case where the parent region is created via cache.xml and the child regions are created dynamically? I believe that could be a valid case. The right thing to do is recover what you can, when you can. Not to make parent recovery dependant on its children. -- Mike Stolz Principal