You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "jchen21 (GitHub)" <gi...@apache.org> on 2018/12/05 00:56:08 UTC
[GitHub] [geode] jchen21 opened pull request #2950: GEODE-6142: Check
JDBC mapping before destroy region
Co-authored-by: Darrel Schneider <ds...@pivotal.io>
Co-authored-by: Jianxia Chen <jc...@apache.org>
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/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jinmeiliao commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
same here, you can use
```suggestion
RegionMapping mapping = CacheElement.findElement(cacheConfig.getCustomRegionElements(), "jdbc-mapping")
```
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
will do
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jinmeiliao commented on issue #2950: GEODE-6142:
Check JDBC mapping before destroy region
Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
My concern is that this would make the region commands aware of a specific extension point, which makes it not extensible after all. JDBC is an extension of the core, I guess the idea is some customer might install geode without installing this extension. Is it right for the core to depend on jdbc code?
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] agingade commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "agingade (GitHub)" <gi...@apache.org>.
Why is this in gfsh command only? What happens when region is destroyed using region api? Can we destroy jdbc-mapping when region is destroyed. We do this for other functionalities, like CQs, indexes, cache writers, loaders.
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
will do
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] agingade commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "agingade (GitHub)" <gi...@apache.org>.
What happens when region is destroyed with API and it has jdbc-mapping?
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
Because the only public way we have of destroying a jdbc-mapping is a gfsh command. All those other examples also have public apis that let you remove the other thing. Although at least some of your examples do not prevent a region from being destroyed (cache writer and loader; I don't know about cqs and indexes). The goal here is to help the gfsh user but not the api user.
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jchen21 commented on issue #2950: GEODE-6142: Check
JDBC mapping before destroy region
Posted by "jchen21 (GitHub)" <gi...@apache.org>.
@BenjaminPerryRoss @gemzdude @monkeyherder Please review the PR. Thanks!
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] jinmeiliao commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "jinmeiliao (GitHub)" <gi...@apache.org>.
I believe you can use
```suggestion
RegionConfig regionConfig = CacheElement.findElement(cacheConfig.getRegions, regionName)
```
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal closed pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
[ pull request closed by dschneider-pivotal ]
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on issue #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
This does not make the geode core depend on jdbc code. It just make it aware of it. If jdbc is not installed then destroy region will still work fine. All we do is look for a xml element that has a certain id.
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] dschneider-pivotal commented on pull request #2950:
GEODE-6142: Check JDBC mapping before destroy region
Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
apis do not change cluster config. Other than that api region destroy does what it always has done. The only additional thing you would want to remove when using the apis is the async queue on the cache. Of course this is already true for the apis; if the region is the last one using the queue then destroy region does not remove that queue.
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org