You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/03/16 03:05:50 UTC
[GitHub] [tinkerpop] heljoyLiu opened a new pull request #1264:
gremlin-python: add session mode client
heljoyLiu opened a new pull request #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1264: gremlin-python:
add session mode client
Posted by GitBox <gi...@apache.org>.
spmallette commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-607317401
@heljoyLiu If you'd like to try to include the documentation changes I suggested above on this PR that would be great, otherwise I will look to add them myself on merge. Also note that you intended to fix this in Javascript:
https://github.com/apache/tinkerpop/pull/1251/files#diff-9ec1348d608a722cece8543e6db87a23R385
If you'd like to add that here on this PR that will be fine, though for some reason you target `3.4-dev` with this PR but all other languages went to `3.3-dev` - I think this change should be re-targetted to `3.3-dev` to be consistent at this point. Please let me know if you think differently for some reason.
As a reminder to myself, on the merge of this one to `master` we probably need to review the impact of:
https://issues.apache.org/jira/browse/TINKERPOP-2336
https://github.com/apache/tinkerpop/blob/0a7769eea651ab0e72d3a6a0bd424fbb1197bd47/docs/src/upgrade/release-3.5.x.asciidoc#session-close
in the context of python/js/.net merging to `master`. Ultimately we want consistency of behavior across all the drivers, so these should be modified to match java (i.e. remove the send of the close message and validate that the close of the channel kills the session).
Well, I hope I've captured all the loose ends on this "session" feature. If anyone has anything else they might add please comment. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu closed pull request #1264: gremlin-python:
add session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu closed pull request #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu commented on issue #1264: gremlin-python: add
session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-607611229
> If you'd like to add that here on this PR that will be fine, though for some reason you target `3.4-dev` with this PR but all other languages went to `3.3-dev` - I think this change should be re-targetted to `3.3-dev` to be consistent at this point. Please let me know if you think differently for some reason.
ok, I will re-target to `3.3-dev` branch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] spmallette commented on issue #1264: gremlin-python:
add session mode client
Posted by GitBox <gi...@apache.org>.
spmallette commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-607311104
I referenced this one from #1263 (`3.4-dev`)which added sessions for .NET. it is linked with #1257 (`3.3-dev`) and #1251 (javascript) With the addition of this PR it would mean that we have session support across all internally managed drivers. So far, I'm not sure if any of these changes have added any documentation and that needs to be rectified with the merge of this one.
Ideally we need:
1. CHANGELOG entry.
2. Upgrade documentation.
3. Adjust text of "[Considering Sessions](https://tinkerpop.apache.org/docs/current/reference/#sessions)" section in the Reference Documentation and add examples in each language.
I think for the first two items we can just refer to this feature generally - no need to have one changelog/upgrade section per language.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu commented on a change in pull request #1264:
gremlin-python: add session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu commented on a change in pull request #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#discussion_r402041901
##########
File path: gremlin-python/src/main/jython/gremlin_python/driver/serializer.py
##########
@@ -53,6 +53,22 @@ def eval(self, args):
return args
+class Session(Processor):
+
+ def authentication(self, args):
+ return args
+
+ def eval(self, args):
+ return args
+
+ def close(self, args):
+ # close session, 'gremlin' in args as tips and not be executed actually
+ gremlin = args.get('gremlin')
+ if not gremlin:
+ args['gremlin'] = 'session.close()'
Review comment:
en. I will remove this like PR in .Net
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu commented on issue #1264: gremlin-python: add
session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-607686757
> I think for the first two items we can just refer to this feature generally - no need to have one changelog/upgrade section per language.
I see, another PR for `3.3-dev` is ready, and I will make one for JavaScript for `close message` too.
To the end, update the first two items
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu commented on issue #1264: gremlin-python: add
session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-599416185
hi @spmallette , it's for `gremlin-python`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1264:
gremlin-python: add session mode client
Posted by GitBox <gi...@apache.org>.
spmallette commented on a change in pull request #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#discussion_r401697847
##########
File path: gremlin-python/src/main/jython/gremlin_python/driver/serializer.py
##########
@@ -53,6 +53,22 @@ def eval(self, args):
return args
+class Session(Processor):
+
+ def authentication(self, args):
+ return args
+
+ def eval(self, args):
+ return args
+
+ def close(self, args):
+ # close session, 'gremlin' in args as tips and not be executed actually
+ gremlin = args.get('gremlin')
+ if not gremlin:
+ args['gremlin'] = 'session.close()'
Review comment:
As in the other languages, I'd prefer we not include any extra arguments to the "close" message. Please modify this python PR to match what was done on the other language PRs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [tinkerpop] heljoyLiu commented on issue #1264: gremlin-python: add
session mode client
Posted by GitBox <gi...@apache.org>.
heljoyLiu commented on issue #1264: gremlin-python: add session mode client
URL: https://github.com/apache/tinkerpop/pull/1264#issuecomment-607687137
thanks @spmallette, I will close this PR
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services