You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "gesterzhou (GitHub)" <gi...@apache.org> on 2019/01/19 00:17:28 UTC
[GitHub] [geode] gesterzhou opened pull request #3100: GEODE-6273:
add server group parameter for creating mapping command
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:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] 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)?
### 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.
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We didn't end up needing the changes made to this file for our work, but the above refactor is an improvement and we should keep it.
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] gesterzhou closed pull request #3100: GEODE-6273:
add server group parameter for creating mapping command
Posted by "gesterzhou (GitHub)" <gi...@apache.org>.
[ pull request closed by gesterzhou ]
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We shouldn't need to stop(3) since there should only be 1 locator and 2 servers.
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
reword this to support multiple groups
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We can remove these comments
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
This method can be removed since we no longer use it
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jchen21 commented on issue #3100: GEODE-6273: add
server group parameter for creating mapping command
Posted by "jchen21 (GitHub)" <gi...@apache.org>.
The failure `org.apache.geode.internal.cache.wan.parallel.ParallelWANStatsDUnitTest > testPartitionedRegionParallelPropagation_AfterDispatch_Redundancy_3 FAILED` is flaky.
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
why not do what was done in create? That is, when groups is null create it with just "cluster".
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We don't need to change this file
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We talked about this, and we think we probably don't need to store this in the cache config. We can remove the code to do so.
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] BenjaminPerryRoss commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "BenjaminPerryRoss (GitHub)" <gi...@apache.org>.
We can remove this commented code
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jchen21 commented on pull request #3100: GEODE-6273:
add server group parameter for creating mapping command
Posted by "jchen21 (GitHub)" <gi...@apache.org>.
Agree with @dschneider-pivotal
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal closed pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
[ pull request closed by dschneider-pivotal ]
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #3100:
GEODE-6273: add server group parameter for creating mapping command
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
I'm concerned that you modified the xsd and added "groups" as an attribute to the RegionMapping.
Region does not have such an attribute; and yet regions are created in a group.
What I expected was no change to the xsd or the RegionMapping class.
I thought you would enhance each of the gfsh commands that work with jdbc-mapping to have a --groups option that would scope that command to only look/store in the given group(s).
[ Full content available at: https://github.com/apache/geode/pull/3100 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org