You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Sai Boorlagadda <sb...@pivotal.io> on 2015/11/25 19:14:38 UTC
Review Request 40715: GEODE-578: Allow cluster-config to work with
cache.xml
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40715/
-----------------------------------------------------------
Review request for geode and anilkumar gingade.
Repository: geode
Description
-------
Before starting a cache-server or creating a region, verify that one has
not been created by cluster config. skip if already created.
fixing compilation errors introduced by the merge.
reverting "skipping region creation" change
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java 14076b530b43a5b7087df33cecfea45e23ae03be
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreationJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40715/diff/
Testing
-------
Thanks,
Sai Boorlagadda
Re: Review Request 40715: GEODE-578: Allow cluster-config to work with
cache.xml
Posted by Sai Boorlagadda <sb...@pivotal.io>.
> On Nov. 25, 2015, 6:59 p.m., Darrel Schneider wrote:
> > gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java, line 566
> > <https://reviews.apache.org/r/40715/diff/1/?file=1147088#file1147088line566>
> >
> > change "startBridgeServers" to "startCacheServers"
> >
> > Anytime you see "bridge" in client/server code change it.
> >
> > Why do we pass "this.getCacheServers()" to an instance method? Does it make unit testing easier?
Yes. To be able to unit test by passing a list of cache servers.
- Sai
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40715/#review108040
-----------------------------------------------------------
On Nov. 25, 2015, 7:34 p.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40715/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2015, 7:34 p.m.)
>
>
> Review request for geode and anilkumar gingade.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Before starting a cache-server or creating a region, verify that one has
> not been created by cluster config. skip if already created.
>
> fixing compilation errors introduced by the merge.
>
>
> reverting "skipping region creation" change
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java 14076b530b43a5b7087df33cecfea45e23ae03be
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreationJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40715/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 40715: GEODE-578: Allow cluster-config to work with
cache.xml
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40715/#review108040
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java (line 562)
<https://reviews.apache.org/r/40715/#comment167422>
change "startBridgeServers" to "startCacheServers"
Anytime you see "bridge" in client/server code change it.
Why do we pass "this.getCacheServers()" to an instance method? Does it make unit testing easier?
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java (line 581)
<https://reviews.apache.org/r/40715/#comment167417>
declarativeCacheServer should be named declarativeCacheServers
This method may modify the contents of the declarativeCacheServer list. At the very least the method should document this with a method comment.
It should also clarify what cache servers it is going to start. Seems like it can get them from both this list and from the cache.
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java (line 610)
<https://reviews.apache.org/r/40715/#comment167420>
change the name "bridge" to something like "declaredCacheServer".
- Darrel Schneider
On Nov. 25, 2015, 10:17 a.m., Sai Boorlagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40715/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2015, 10:17 a.m.)
>
>
> Review request for geode and anilkumar gingade.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Before starting a cache-server or creating a region, verify that one has
> not been created by cluster config. skip if already created.
>
> fixing compilation errors introduced by the merge.
>
>
> reverting "skipping region creation" change
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java 14076b530b43a5b7087df33cecfea45e23ae03be
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreationJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40715/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Sai Boorlagadda
>
>
Re: Review Request 40715: GEODE-578: Allow cluster-config to work with
cache.xml
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40715/
-----------------------------------------------------------
(Updated Nov. 25, 2015, 7:34 p.m.)
Review request for geode and anilkumar gingade.
Changes
-------
incorporated review feedback
Repository: geode
Description
-------
Before starting a cache-server or creating a region, verify that one has
not been created by cluster config. skip if already created.
fixing compilation errors introduced by the merge.
reverting "skipping region creation" change
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java 14076b530b43a5b7087df33cecfea45e23ae03be
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreationJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40715/diff/
Testing
-------
precheckin
Thanks,
Sai Boorlagadda
Re: Review Request 40715: GEODE-578: Allow cluster-config to work with
cache.xml
Posted by Sai Boorlagadda <sb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40715/
-----------------------------------------------------------
(Updated Nov. 25, 2015, 6:17 p.m.)
Review request for geode and anilkumar gingade.
Repository: geode
Description
-------
Before starting a cache-server or creating a region, verify that one has
not been created by cluster config. skip if already created.
fixing compilation errors introduced by the merge.
reverting "skipping region creation" change
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreation.java 14076b530b43a5b7087df33cecfea45e23ae03be
gemfire-core/src/test/java/com/gemstone/gemfire/internal/cache/xmlcache/CacheCreationJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40715/diff/
Testing (updated)
-------
precheckin
Thanks,
Sai Boorlagadda