You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by galen-pivotal <gi...@git.apache.org> on 2017/03/15 17:43:17 UTC
[GitHub] geode pull request #426: GEODE-2660 Add @Experimental to the Redis adapter.
GitHub user galen-pivotal opened a pull request:
https://github.com/apache/geode/pull/426
GEODE-2660 Add @Experimental to the Redis adapter.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/galen-pivotal/incubator-geode feature/GEODE-2660
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/426.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #426
----
commit 23dc29d909ab5127b8e717555bbc564bbbe1e88e
Author: Galen OSullivan <go...@pivotal.io>
Date: 2017-03-15T17:41:42Z
GEODE-2660 Add @Experimental to the Redis adapter.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
@hiteshk25 @kirklund @upthewaterspout @bschuchardt
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
@bschuchardt want to merge this?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/geode/pull/426
@galen-pivotal - it looks like the public API is just GeodeRedisServer.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
@bschuchardt This is just the experimental annotation, not changes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/geode/pull/426
I think adding @Experimental the the internal classes might be overkill, @Experimental really just has meaning for the public API.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/426
I don't think we can merge these changes until the lock-service variables in ExeuctionHandlerContext are made static. As it's currently implemented every ExecutionContextHandler has its own lock service, so concurrent operations aren't protected. This also calls into question any performance figures obtained with this code since it isn't functioning properly.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
I can't merge as I'm not a committer; would someone else be so kind, assuming these changes look 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
@upthewaterspout okay, you'd just put it on GeodeRedisServer?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/geode/pull/426
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by galen-pivotal <gi...@git.apache.org>.
Github user galen-pivotal commented on the issue:
https://github.com/apache/geode/pull/426
@bschuchardt Check it out again; I force-pushed with just the annotation on GeodeRedisServer.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/426
my bad - I checkout out ggreen's branch by mistake. I don't see any @Experimental at all in your branch. It looks like your second commit removed it from GeodeRedisServer.java when you meant to only remove it from the other classes.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/426
got it
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode issue #426: GEODE-2660 Add @Experimental to the Redis adapter.
Posted by bschuchardt <gi...@git.apache.org>.
Github user bschuchardt commented on the issue:
https://github.com/apache/geode/pull/426
Also, since we're talking about releasing v 1.2 soon it would be nice to add the Experimental annotation with this same commit since it is breaking compatibility with v 1.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 if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---