[GitHub] tinkerpop issue #865: TINKERPOP-1963 Fixed branch() problems with reducing s...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/865 VOTE +1 ---
[GitHub] tinkerpop issue #870: TINKERPOP-1968 Refactor Gremlin Server integration tes...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/870 VOTE +1 ---
[GitHub] tinkerpop issue #869: TINKERPOP-1841 Configure python tests in travis
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/869 VOTE +1 ---
[GitHub] tinkerpop issue #854: TINKERPOP-1933 gremlin-python maximum recursion depth ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/854 My bad on the lack of merge. I'll do it today. ---
[GitHub] tinkerpop issue #854: TINKERPOP-1933 gremlin-python maximum recursion depth ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/854 I'm happy to write a test. Is there an established way to to test large streaming responses? Or any tips on how to write one? ---
[GitHub] tinkerpop issue #854: TINKERPOP-1933 gremlin-python maximum recursion depth ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/854 VOTE +1 ---
[GitHub] tinkerpop pull request #854: TINKERPOP-1933 gremlin-python maximum recursion...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/854 TINKERPOP-1933 gremlin-python maximum recursion depth exceeded on large responses https://issues.apache.org/jira/browse/TINKERPOP-1933 Recursive calls to the `data_received` were causing max recursion depth error with large streaming responses. This PR remove the recursion in favor of a simple loop. I didn't add any new tests. Locally, I tested like this: ```python from gremlin_python.driver import client groovy = """ def num_nodes = 1..10 for (n in num_nodes) { g.addV().next() } """ c = client.Client('ws://localhost:8182/gremlin', 'g') results = c.submit(groovy) results.all().result() results = c.submit("g.V().count()") print("Added %d nodes to graph" % results.all().result()[0]) # This was always ok print("Ok up to len: %d" % len(c.submit("g.V().limit(61888)").all().result())) try: # This used to throw error print("Retrieved %d nodes, no more recursion issue" % len(c.submit("g.V().limit(10)").all().result())) except: pass ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1933 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/854.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 #854 commit 4c8717dd4e94ec248a959911d8c11f3b45b2d7b3 Author: davebshow Date: 2018-04-23T23:21:48Z Don't use recursive calls for streaming response ---
[GitHub] tinkerpop issue #796: Added correct setting of future exceptions, pursuant t...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/796 sweet ---
[GitHub] tinkerpop issue #796: Added correct setting of future exceptions, pursuant t...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/796 Yes this affects both 32 and master ---
[GitHub] tinkerpop issue #797: TINKERPOP-1887 Allow to use bindings in predicates
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/797 There is a bit of a learning curve working with the Gremlin-Python source. Do you plan on giving this another try? If so, I will help you review. If not, let me know and I can give it a go. ---
[GitHub] tinkerpop issue #797: TINKERPOP-1887 Allow to use bindings in predicates
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/797 https://github.com/apache/tinkerpop/tree/master/gremlin-python/src/main/jython/tests ---
[GitHub] tinkerpop issue #797: TINKERPOP-1887 Allow to use bindings in predicates
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/797 This needs a test to demonstrate that it works. I haven't run the code yet, but what happens to the key value of the binding? Does this generate GraphSON with bindings, or just the values of the bindings? ---
[GitHub] tinkerpop issue #796: Added correct setting of future exceptions, pursuant t...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/796 LGTM VOTE +1 ---
[GitHub] tinkerpop issue #790: TINKERPOP-1875 Gremlin-Python only aggregates to list ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/790 VOTE +1 ---
[GitHub] tinkerpop pull request #790: TINKERPOP-1875 Gremlin-Python only aggregates t...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/790#discussion_r165509842 --- Diff: gremlin-python/src/main/jython/tests/conftest.py --- @@ -1,4 +1,4 @@ -''' +""" --- End diff -- Ok willdo. ---
[GitHub] tinkerpop pull request #790: TINKERPOP-1875 Gremlin-Python only aggregates t...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/790#discussion_r164506631 --- Diff: gremlin-python/src/main/jython/tests/conftest.py --- @@ -1,4 +1,4 @@ -''' +""" --- End diff -- When possible, I try to follow the style guidelines laid out in [PEP 8](https://www.python.org/dev/peps/pep-0008/#string-quotes). Docstrings specifically relate to [PEP 257](https://www.python.org/dev/peps/pep-0257/). Gremlin Python wasn't originally written with these guidelines in mind, so sometimes I make little style fixes in code that I am updating. Maybe not the best idea, since it doesn't really relate to the PR. ---
[GitHub] tinkerpop pull request #790: TINKERPOP-1875 Gremlin-Python only aggregates t...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/790 TINKERPOP-1875 Gremlin-Python only aggregates to list when using GraphSON3 https://issues.apache.org/jira/browse/TINKERPOP-1875 This PR fixes server response message deserialization in Gremlin-Python. Improper deserialization was leading to `aggregate_to` always resulting in a list as well as API comparability issues between GraphSON2 and GraphSON3 message serializers. This PR also improves testing--all Gremlin-Python tests using a remote connection are now run with both GraphSON 2 and 3 serializers. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1875 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/790.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 #790 commit f0bbe7c90f5f66028203f21dcf720bf0948f1950 Author: davebshow Date: 2018-01-24T21:14:03Z added proper response message serialization, run all remote connection tests with graphson 2 and 3 ---
[GitHub] tinkerpop issue #769: TINKERPOP-1844 Default GraphSON 3.0 for GLV tests in g...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/769 VOTE +1 ---
[GitHub] tinkerpop issue #749: Tinkerpop 1807 Gremlin-Python doesn't support GraphSON...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/749 Done. VOTE +1 ---
[GitHub] tinkerpop issue #749: Tinkerpop 1807 Gremlin-Python doesn't support GraphSON...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/749 Does this need a changelog for 3.3.1 as well, or are the 3.2.7 changes assumed to be included in 3.3.1? ---
[GitHub] tinkerpop pull request #749: Tinkerpop 1807 Gremlin-Python doesn't support G...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/749 Tinkerpop 1807 Gremlin-Python doesn't support GraphSON types g:Date, g:Timestamp and g:UUID https://issues.apache.org/jira/browse/TINKERPOP-1807 This PR follows #741, adding support for core types Date, Timestamp, and UUID in GraphSON3 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1807-master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/749.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 #749 commit 3b651ff58cdf210a63a0101d0a311c7076de2b0e Author: davebshow Date: 2017-11-01T23:31:10Z Implemented support for missing core GraphSON types in gremlin-python commit 4661284292306fecb67fbcc46617af97b3b7762f Author: davebshow Date: 2017-11-07T17:40:05Z updated changelog and upgrade docs commit c0078cc172e918f96460630da27ff04cb4e9478d Author: davebshow Date: 2017-11-13T18:11:46Z added asserts to check values are deserialized as expected commit 5031ccdbea28b1384c40fd522e9fec25b5b86818 Author: davebshow Date: 2017-11-15T18:12:22Z added core GraphSON types UUID, Date, and Timestamp for GraphSON 3 commit ed6133e502e573650a39a6ac96c4a901e168c2e6 Author: davebshow Date: 2017-11-15T18:17:34Z fixed typo in changelog commit 80624fcbe34ffb0d4d2a4033926a198096413ae3 Author: davebshow Date: 2017-11-15T18:19:59Z removed unwanted blank line ---
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/741#discussion_r150620490 --- Diff: gremlin-python/src/main/jython/tests/structure/io/test_graphson.py --- @@ -121,6 +125,19 @@ class X(object): serdes.objectify.assert_called_once_with(value, reader) assert o is serdes.objectify() +def test_datetime(self): +dt = self.graphson_reader.readObject(json.dumps({"@type": "g:Date", "@value": 1481750076295})) +assert isinstance(dt, datetime.datetime) --- End diff -- Done ---
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/741#discussion_r150589566 --- Diff: gremlin-python/src/main/jython/tests/structure/io/test_graphson.py --- @@ -121,6 +125,19 @@ class X(object): serdes.objectify.assert_called_once_with(value, reader) assert o is serdes.objectify() +def test_datetime(self): +dt = self.graphson_reader.readObject(json.dumps({"@type": "g:Date", "@value": 1481750076295})) +assert isinstance(dt, datetime.datetime) --- End diff -- Yeah I guess. TBH, I don't find any of these tests too meaningful, which is why I added the functional IO tests at the end that actually send and receive messages from the server and verify that they are equal. I can add the asserts though if you want. ---
[GitHub] tinkerpop pull request #741: TINKERPOP-1807 Gremlin-Python doesn't support G...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/741 TINKERPOP-1807 Gremlin-Python doesn't support GraphSON types g:Date, g:Timestamp and g:UUID https://issues.apache.org/jira/browse/TINKERPOP-1807 This PR add support for GraphSON types `UUID`, `Date`, and `Timestamp` as well as associated tests. `UUID` and `Date` are straightforward, as the Python standard library includes `Date` (`datetime.datetime`) and `UUID` (`uuid.UUID`) objects. However, in Python, a timestamp is just a float, so in order to support serialization to a GraphSON type Timestamp, I added a dummy class `timestamp` to the `statics` module, similar to the `long` class for Python 3. Note, another PR will be required for master with implementations for both GraphSON2 and 3. I will open upon approval of this one. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1807 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/741.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 #741 commit 3b651ff58cdf210a63a0101d0a311c7076de2b0e Author: davebshow Date: 2017-11-01T23:31:10Z Implemented support for missing core GraphSON types in gremlin-python ---
[GitHub] tinkerpop issue #728: Do not strong-freeze dependencies
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/728 VOTE +1 ---
[GitHub] tinkerpop pull request #691: TINKERPOP-1747 Streamline inheritance for greml...
Github user davebshow closed the pull request at: https://github.com/apache/tinkerpop/pull/691 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/691 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/691 Yeah well there are different approaches we could have taken, e.g. leaving the custom reader/writer options for the v2 and v3 serializer classes, but it seems to me that the point of those classes is to provide consistency. In the less common use case where the user provides reader/writer I think that it is reasonably to expect that they can deal with all the required configuration using `GraphSONMessageSerializer`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #691: TINKERPOP-1747 Streamline inheritance for greml...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/691 TINKERPOP-1747 Streamline inheritance for gremlin-python GrapnSON serializer classes https://issues.apache.org/jira/browse/TINKERPOP-1747 This PR cleans up the code used to define GraphSON serializer classes in gremlin-python. `GraphSONMessageSerializer` acts as a base class for `GraphSONSerializersV2d0` and `GraphSONSerializersV3d0` in a similar fashion, but with this change: 1. While `GraphSONMessageSerializer` can accept custom reader, writer, and version implementations in order to create a custom serializer, `GraphSONSerializersV2d0` and `GraphSONSerializersV3d0` cannot. 2. Instead `GraphSONSerializersV3d0` and `GraphSONSerializersV2d0` are in a sense implementations of custom serializers, and they always use `graphsonV2d0.GraphSONReader()`/`graphsonV2d0.GraphSONWriter()` and `graphsonV3d0.GraphSONReader()`/`graphsonV3d0.GraphSONWriter()` respectively. As before, `GraphSONMessageSerializer` will default to the current version of GraphSON (v3 for TP 3.3.0) if no custom reader/writer implementation is passed. Defaults are now specified in a highly visible class attribute position using uppercase naming like class CONSTANTS. All other changes, like using `super` to call parent class's init methods, are more stylistic, reflecting current Python standards for best practice. I also added a couple small tests to make sure each serializer was assigned proper reader, writer classes as well as version information. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1747 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/691.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 #691 commit 83206a2101a0f03c515d4e1829149ed14a330dc7 Author: davebshow Date: 2017-08-09T19:14:44Z cleaned up gremlin-python message serializer 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #678: TINKERPOP-1715: update spark version to 2.2
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/678 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #675: Ensure serializers iteration is consistent
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/675 Vote +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #:
Github user davebshow commented on the pull request: https://github.com/apache/tinkerpop/commit/f99f5524183891372ae7ac60651ff8ac7d23e7d7#commitcomment-22104442 In gremlin-python/src/main/jython/tests/process/test_dsl.py: In gremlin-python/src/main/jython/tests/process/test_dsl.py on line 41: I think this is good except that all the methods for the anonymous DSL traversal should return the corresponding DSL traversal. Maybe an approach similar to the one used with SocialTraversalSource, where the `graph_traversal` attribute is assigned as a class attribute would be best: ```python class ___(__): graph_traversal = SocialTraversal ``` The static methods would then need to be changed to class methods, and would look like: ```python @classmethod def knows(cls, person_name): return cls.graph_traversal(None, None, Bytecode()).knows(person_name) ``` This would of course require changes to the groovy code that generates the `__` class. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #594: TINKERPOP-1577 Building gremlin-python with python 2/3
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/594 Tested using mvn clean install with .glv file. Gremlin tests ran successfully against Python 2.7 and Python 3.5 interpreter. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #594: TINKERPOP-1577 Building gremlin-python with python 2/3
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/594 Should this include documentation on how to run against the different interpreters? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #591: TINKERPOP-1663 Validation for maximum number of parame...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/591 LGTM. Also ran server/console integration. Docs are clear. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #593: TINKERPOP-1665 Remove unittest from Gremlin-Python tes...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/593 Didn't even know that existed. Done. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #593: TINKERPOP-1665 Remove unittest from Gremlin-Pyt...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/593 TINKERPOP-1665 Remove unittest from Gremlin-Python tests A simple PR that removes unnecessary `unittest` code from the Gremlin-Python test suite. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1665 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/593.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 #593 commit d06868292d7f2592772dac2f15281fe2803cc328 Author: davebshow Date: 2017-04-05T02:02:08Z removed unittest from gremlin-python tests --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/565 `docker/build.sh -t -n -i` succeeded. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #554: TINKERPOP-1599 implement real gremlin-python driver
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/554 Merged. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #554: TINKERPOP-1599 implement real gremlin-python dr...
Github user davebshow closed the pull request at: https://github.com/apache/tinkerpop/pull/554 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #554: TINKERPOP-1599 implement real gremlin-python driver
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/554 I think this is ready. Anyone want to be a third reviewer for this? VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #558: TINKERPOP-1631 Improvements to BindingsGremlinPlugin
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/558 `mvn clean install` passes. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #561: TINKERPOP-1635 Fix duplicate serialization of element ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/561 Yeah it looks fine to me. It seems that properties are serialized before the element dict is "cleaned up". I don't see why you would serialize it again. However, I haven't really familiarized myself with the new GraphSON work on master yet. Still, I see no reason why we can't merge this. If the code is refactored and it is no longer relevant, so be 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #554: TINKERPOP-1599 implement real gremlin-python driver
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/554 Well, I've made my last review of this code and pushed a bit of cleanup and a small fix. IMHO, this is ready to be merged. I'll wait to see if there are any more comments or feedback before upvoting. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #554: TINKERPOP-1599 implement real gremlin-python driver
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/554 Ok, I have now rebased ad added docs to both reference and upgrading. I also reviewed the old implementation and made a couple small fixes for consistency. I also added standard op processor message serialization. I think this one is getting close to being ready to go. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #554: TINKERPOP-1599 implement real gremlin-python driver
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/554 Yeah I figured we would need to add docs. Where should I add to the reference docs? Maybe in [Gremlin Applications](https://github.com/apache/tinkerpop/blob/master/docs/src/reference/gremlin-applications.asciidoc) after the [Connecting via Java](https://github.com/apache/tinkerpop/blob/master/docs/src/reference/gremlin-applications.asciidoc#connecting-via-java) section? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #554: TINKERPOP-1599 implement real gremlin-python dr...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/554 TINKERPOP-1599 implement real gremlin-python driver https://issues.apache.org/jira/browse/TINKERPOP-1599 This PR adds a better driver implementation for gremlin-python: - Uses a multi-threaded solution to provide asynchronous I/O - Decouples the underlying websocket client implementation from driver code, making it easy to plug in a different client - Makes it easy to plug in different protocols, like the Gremlin Server HTTP protocol. - Adds simple connection pooling used in concurrent requests, which increases driver performance with slow traversals - Improves driver tests by adding `pytest` fixtures and removing unneeded `unittest` code. This driver still isn't full featured compared to the Java driver, for example, it doesn't implement `Cluster` to use multiple hosts. But, it is considerably better than the old implementation, and as this was becoming a big PR, I figure we can add more features with subsequent PRs. Also note, I know we are in code freeze right now, so I don't expect this to be reviewed/merged for 3.2.4, but I'm busy and I wanted to get this done so I can move on. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1599 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/554.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 #554 commit b10d6048ced6de08c307b8b9382d67e97e2cb0ef Author: davebshow Date: 2017-01-30T22:22:36Z added code for new driver, updated driver tests --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #553: remove tests from setup.py
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/553 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #551: Fix the serializers lookup to handle the type first, t...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/551 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #551: Fix the serializers lookup to handle the type first, t...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/551 This makes a lot of sense. You can probably remove the instance check for the custom long type after making this change: https://github.com/apache/tinkerpop/blob/master/gremlin-python/src/main/jython/gremlin_python/structure/io/graphson.py#L346-L348 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #533: TINKERPOP-1600 Added base64 encoded string to sasl cha...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/533 All of my driver tests succeed against this branch. They use `'saslMechanism': 'PLAIN'`, so they don't expect any `sasl` value to be returned. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #515: TINKERPOP-1581 Gremlin-Python driver connection...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/515 TINKERPOP-1581 Gremlin-Python driver connection is not thread safe. https://issues.apache.org/jira/browse/TINKERPOP-1581 This was a simple fix. The `DriverRemoteConnection.__init__` method no longer ignores the `loop` kwarg passed by user. I added two tests using threads as well. One demonstrates how not passing an `IOLoop`, which was in effect the behavior before this PR, will cause a `RuntimeError`. The other shows how you can now pass an `IOLoop` for each thread to avoid this problem. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1581 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/515.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 #515 commit 2f3386daed8323b77aa48ed92335f353d09d06c1 Author: davebshow Date: 2016-12-09T21:40:53Z allow user to pass loop to DriverRemoteConnection. added multi-threaded tests. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 Tests pass. I think this is a good base to move forward to a fully functional async API for Gremlin. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 Well I implemented this API for gremlin-python. There is one major problem: side effects. Since the current side effect API is designed to be synchronous (calling `run_sync`), it cannot be used inside a coroutine, as demonstrated by this [test](https://github.com/apache/tinkerpop/blob/54fac5146a5ac067e75f2fbfe135fae2da641c35/gremlin-python/src/main/jython/tests/driver/test_driver_remote_connection.py#L210). This is problematic for a Tornado or Asyncio based driver. I think in the future we should consider the possibility that the `promise()` method return a `AsyncSideEffect` object so the end user can leverage the async techniques with side effects. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 As long as the driver returns a future with a compatible API, yes, it can be used with all Python versions. The snippet you provided will throw an error with an Asyncio or Tornado client, as you can only call [`result()`](https://docs.python.org/3/library/asyncio-task.html#asyncio.Future.result) on a completed future. You will typically need to `yield/yield from/await` it first depending on the Python version and driver (like in the examples provided above). However, the underlying coroutine will spawn independently, which means that even if you don't `yield/yield from/await`, but you wait long enough before calling `result()` it will complete. For example: ```python future = g.V().promise(lambda t: t.toList()) assert not future.done() # Future has not completed await asyncio.sleep(5) # Simulate doing a ton of other blocking stuff assert future.done() # Future has completed (assuming 5 seconds was enough time to complete) result = future.result() # Doesn't throw an error ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #505: TINKERPOP-1561 gremiln-python GraphSONWriter do...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/505 TINKERPOP-1561 gremiln-python GraphSONWriter doesn't properly serialize long in Python 3.5 https://issues.apache.org/jira/browse/TINKERPOP-1561 Added custom `dictify()` method to `Int32IO` that checks for long type. This ensures proper serialization of `long` when using Python 3. Originally in the Jira ticket I proposed a different approach, but after looking more closely this is the simplest approach. VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1561 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/505.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 #505 commit 8ab88820b263fa3d800fbf38bebf2a4232a01607 Author: davebshow Date: 2016-11-26T19:43:54Z check for long type in Int32 serializer --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #:
Github user davebshow commented on the pull request: https://github.com/apache/tinkerpop/commit/aa85a9b5278c55aa28014aa135c8498428295071#commitcomment-19905983 In gremlin-python/src/main/jython/gremlin_python/driver/driver_remote_connection.py: In gremlin-python/src/main/jython/gremlin_python/driver/driver_remote_connection.py on line 62: It is also worth noting that this example would require more refactoring here in the remote connection to send the request to the server before generating the future. The example here is more aimed at the GLV promise method implementation. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 I put together a quick example of how this can be implemented in gremlin-python. Obviously the example is incomplete, but hopefully it can help move the discussion forward: https://github.com/apache/tinkerpop/commit/aa85a9b5278c55aa28014aa135c8498428295071 These changes result in the following APIs depending on which Python future is returned by the driver, but the GLV doesn't care as long as it supports Python's common future API: - Tornado w/Python 2.7+ returning a `tornado.concurrent.Future` ```python @gen.coroutine def go(): vertices = yield g.V().promise(lambda x: x.toList()) assert len(vertices) == 6 count = yield g.V().count().promise(lambda x: x.next()) assert count == 6 loop.run_sync(go) ``` - Asyncio/Tornado with Python 3.5 async/await syntax returning `asyncio.Future` or `tornado.concurrent.Future` ```python async def go(): vertices = await g.V().promise(lambda x: x.toList()) assert len(vertices) == 6 count = await g.V().count().promise(lambda x: x.next()) assert count == 6 loop.run_until_complete(go()) ``` - Driver with Python 2.7+ that returns `concurrent.futures.Futures` (or backport) ```python def go(): vertices = g.V().promise(lambda x: x.toList()).result() assert len(vertices) == 6 count = g.V().count().promise(lambda x: x.next()).result() assert count == 6 ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 I agree with the `RemoteConnection` implementation being pluggable. Therefore, the GLV should be able to use any remote connection as long as it complies with the API. But, since pre-Python3 doesn't have any standard implementation of a `Future`, in order to maintain this "plugablity", IMO the GLV should be able to handle whatever type of `Future` the `RemoteConnection` returns. This places the onus on the end user to understand what `RemoteConnection` implementation they are using and the type of Future returned in order to comply with the syntax required by underlying framework implementation. This does not mean that responses need have a different API, only that developers know how to either `yield` or `yield from` them, or use the `concurrent.futures.as_completed` method. The good thing is that the three possible future types (asyncio, tornado, and concurrent) have compatible APIs, and should be able to all be handled by the GLV in the same fashion. However, if I understand correctly (not sure that I do) it seems to me that doing specialized handling, would require us to commit to a certain syntax. Unfortunately, all of these problems really stem from the Python community's unwillingness to drop the legacy Python implementation, which the Python Software Foundation has been urging us to do for years. If I am missing something, or there is a solution that I can't see for some reason, please provide an example of a way that allows any pluggable `RemoteConnection` that can be based on any of the possible Python frameworks that maintain the syntax used for handling the resulting future by the end user. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 @aholmberg I'm not sure if I follow...it seems to me that if a `RemoteConnection` implementation returns a list (or future list ) of `FutureTraversers`, the traversal API can still remain independent from the transport. Even if the GLV needs to operate on the Traversers, `asyncio.Future`, `concurrent.Future`, and `tornado.concurrent.Future` have compatible APIs (`done`, `cancel`, `add_done_callback`, `result`, `set_result`, `etc.`). The "specialized" handling of the future, whether it be a coroutine that requires a `yield`, `yield from`, or `await` expression, or a `concurrent.Future`, will in the end be handled by application code written by the end user. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 After actually thinking about this, the type of future returned will depend on the underlying `RemoteConnection` implementation. A call to the method`promise` will cause a `(async)traversal_strategy` to call `apply`, the result of this depends on the underlying remote connection, and all the GLV has to do is pass the returned futures along to the end user. Therefore a Tornado based driver would return a `tornado.concurrent.Future` etc. etc. Make sense? Or am I missing something...? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/478 @spmallette you can easily implement something similar in Python using Futures. Using `tornado.concurrent.Future` would probably make the most sense, as is is already 2/3 compatible and should work with any Python async syntax (`yield`/`yield from`/`await`). The only problem with is that this ties the GLV code more tightly to Tornado, which was previously only used for IO; however, a similar implementation of Future isn't included with the Python 2 standard library, so Tornado is probably the best option. I actually did something pretty similar to this in the `gremlinclient` library. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #451: TINKERPOP-1458 Gremlin Server doesn't return co...
Github user davebshow closed the pull request at: https://github.com/apache/tinkerpop/pull/451 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #451: TINKERPOP-1458 Gremlin Server doesn't return confirmat...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/451 Manually closing post merge. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #448: Python glv graphson update
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/448 Yeah makes sense. I'm hoping to do some major refactors of the Driver over the next couple months anyway, but I'm juggling too many things right now. Either way this seems good to go. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #448: Python glv graphson update
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/448 I like this. Much simpler, well tested, in general a nice PR. Pretty slick using the metaclass to create the de-/serializer maps. One thing I wonder: Presumably using the `GraphSONIO` instance methods `writeObject`/`toObject` methods instead of the previous classmethods provides an advantage because we can now pass a custom serializer/deserializer map to the `GraphSONIO` instance, thus overriding the default serializers. However, the `GraphSONIO` instance is created inside the `DriverRemoteConnection` without the optional kwargs. I wonder if we should add `graphson_io=None` to the `__init__` method signature. In the case of `None`, we can instantiate the object as is: ``` def __init__(self, url, traversal_source, username="", password="", loop=None, graphson_io=None): if not graphson_io: graphson_io = GraphSONIO() self._graphson_io = graphson_io ``` Other than that and my previous comment. This is LGTM. VOTE + 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #451: TINKERPOP-1458 Gremlin Server doesn't return confirmat...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/451 Alright I think this branch is ready to go. VOTE + 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #451: TINKERPOP-1458 Gremlin Server doesn't return co...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/451#discussion_r82311244 --- Diff: gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java --- @@ -39,12 +40,14 @@ public class DriverRemoteTraversalSideEffects extends AbstractRemoteTraversalSideEffects { private final Client client; -private Set keys = null; +private Set keys = Collections.emptySet(); private final UUID serverSideEffect; private final Host host; private final Map sideEffects = new HashMap<>(); +private boolean closed = false; --- End diff -- How would you remedy this? Is it worth using `AtomicBoolean`? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #448: Python glv graphson update
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/448 To be honest, I think that we should move away from the `unittest` framework in the future. I know that `pytest` allows for unittest integration, but in general this is to facilitate the transition to `pytest` for large projects that already use `unittest`. The main disadvantage of using both frameworks is that many of the great features in `pytest` require more boilerplate if we inherit from `unittest.TestCase`. The other problem is that `unittest` requires that you learn the assertion API, whereas with `pytest` you just use good old fashioned Python. I was going to open a JIRA about this, but I haven't gotten around to it yet. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #451: TINKERPOP-1458 Gremlin Server doesn't return co...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/451#discussion_r82239737 --- Diff: gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffectsTest.java --- @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.driver.remote; + +import org.apache.tinkerpop.gremlin.driver.AbstractResultQueueTest; +import org.apache.tinkerpop.gremlin.driver.Client; +import org.apache.tinkerpop.gremlin.driver.ResultSet; +import org.apache.tinkerpop.gremlin.driver.message.RequestMessage; +import org.apache.tinkerpop.gremlin.process.traversal.TraversalSideEffects; +import org.junit.Test; + +import java.util.UUID; +import java.util.concurrent.CompletableFuture; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author Stephen Mallette (http://stephen.genoprime.com) + */ +public class DriverRemoteTraversalSideEffectsTest extends AbstractResultQueueTest { --- End diff -- Well, I did add one test for `get()` for before and after `close()`: https://github.com/apache/tinkerpop/blob/TINKERPOP-1458/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffectsTest.java#L67-L84 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #451: Tinkerpop 1458 Gremlin Server doesn't return co...
Github user davebshow commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/451#discussion_r82095012 --- Diff: gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversalSideEffects.java --- @@ -54,22 +57,30 @@ public DriverRemoteTraversalSideEffects(final Client client, final UUID serverSi @Override public V get(final String key) throws IllegalArgumentException { if (!sideEffects.containsKey(key)) { -// specify the ARGS_HOST so that the LoadBalancingStrategy is subverted and the connection is forced -// from the specified host (i.e. the host from the previous request as that host will hold the side-effects) -final RequestMessage msg = RequestMessage.build(Tokens.OPS_GATHER) -.addArg(Tokens.ARGS_SIDE_EFFECT, serverSideEffect) -.addArg(Tokens.ARGS_SIDE_EFFECT_KEY, key) -.addArg(Tokens.ARGS_HOST, host) -.processor("traversal").create(); -try { -final Result result = client.submitAsync(msg).get().one(); -sideEffects.put(key, null == result ? null : result.getObject()); -} catch (Exception ex) { -final Throwable root = ExceptionUtils.getRootCause(ex); -if (root.getMessage().equals("Could not find side-effects for " + serverSideEffect + ".")) -sideEffects.put(key, null); -else -throw new RuntimeException("Could not get keys", root); +if (!closed) { +// specify the ARGS_HOST so that the LoadBalancingStrategy is subverted and the connection is forced +// from the specified host (i.e. the host from the previous request as that host will hold the side-effects) +final RequestMessage msg = RequestMessage.build(Tokens.OPS_GATHER) +.addArg(Tokens.ARGS_SIDE_EFFECT, serverSideEffect) +.addArg(Tokens.ARGS_SIDE_EFFECT_KEY, key) +.addArg(Tokens.ARGS_HOST, host) +.processor("traversal").create(); +try { +final Result result = client.submitAsync(msg).get().one(); --- End diff -- Should this be `all` instead of `one`? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #451: Tinkerpop 1458 Gremlin Server doesn't return co...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/451 Tinkerpop 1458 Gremlin Server doesn't return confirmation upon Traversal OpProcessor "close" op https://issues.apache.org/jira/browse/TINKERPOP-1458 This PR updates the Gremlin Server protocol to send a no content confirmation when a client submits a `close` Op with the `traversal` OpProcessor. It also adds close methods to the Java driver `DriverRemoteTraversalSideEffects` class and the gremlin-python `RemoteTraversalSideEffects` class. Furthermore, `RemoteTraversalSideEffects` now caches side effects locally in order to maintain consistent behavior between the two implementations. This functionality is tested using integration tests in both the gremlin-server and gremlin-python module, as well as in the gremlin-driver unit tests with mocked responses. Thanks to @spmallette for helping me through the Java stuff. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1458 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/451.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 #451 commit 8ab7e50824ee72393c23d416ee2ea8348145b4bf Author: davebshow Date: 2016-09-22T18:51:21Z TraversalOpProcessor returns a success message upon receiveing a close command commit 733bd7e29d7da1a4b972d208cb3ef6f32861e67e Author: davebshow Date: 2016-09-26T17:25:17Z added close method to gremlin python sideeffects commit ef553d5f72fe10bfc10342981452a93d75deb203 Author: davebshow Date: 2016-09-29T15:55:09Z added close method to DriverRemoteTraversalSideEffects, implement AutoCloseable on TraversalSideEffects, add test for close method commit e574bbf867e1ec6c7bbba59f1395648e5cd0fc5a Author: davebshow Date: 2016-09-29T16:04:03Z got rid of wildcard set by intellij commit 10779228ba5ed07ab43e84bef458e17fdfb9deb8 Author: davebshow Date: 2016-09-29T17:04:57Z fixed logic in DriverRemoteSideEffects, don't clear local side effect cache commit d60def3d45d2618c681d6b2f88f1a7017b09f407 Author: Stephen Mallette Date: 2016-09-30T13:23:29Z Add some tests for DriverRemoteTraversalSideEffects. commit 790aa060ce828b8d76f90b06f59e770431e7b732 Author: davebshow Date: 2016-10-05T19:01:59Z added integration tests for DriverRemoteTraversalSideEffects methods commit fd2d6eb86f9d45a709d0684a23d57c83c18f5826 Author: davebshow Date: 2016-10-05T21:59:15Z fixed side effect methods and updated tests commit f3baae8ba2415191e700c3842f983001096168e5 Author: davebshow Date: 2016-10-05T22:00:27Z updated driver to cache side effects locally commit ff41aafd2563d59e3fc95bb310708c5ebd61a4ce Author: davebshow Date: 2016-10-05T22:06:02Z removed extra lines commit 89a5f37724833d45618fdc4f7eb6fa22ddaaf783 Author: davebshow Date: 2016-10-05T22:10:15Z updated changelog --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #437: TINKERPOP-790 TraversalSource and Traversal implement ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/437 Good stuff, I should get there today. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #418: Fixed issue in NumberSerializer that could cause integ...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/418 I agree, the tests need work. I'm hoping to get there this weekend. I think though, as Stephen said, this can be merged pretty safely. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #413: TINKERPOP-1442 Improved session cleanup on client clos...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/413 Stop accepting transactions after a close makes sense to me. This branch builds successfully using mvn clean install. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #412: TINKERPOP-1442 Improved session cleanup on client clos...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/412 This branch builds successfully using mvn clean install. VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #415: TINKERPOP-1448 gremlin-python should be Python 2/3 com...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/415 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #416: TINKERPOP-1449 Streamline gremlin-python build
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/416 Worked like a charm. Vote +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #415: TINKERPOP-1448 gremlin-python should be Python 2/3 com...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/415 1c5e698 add simple package metadata. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #415: Tinkerpop 1448: gremlin-python should be Python 2/3 co...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/415 Maintaining compatibility should be pretty simple, but as you can see from the PR, there are little things that can go wrong. I think the main issue we will run into with this library is numeric typing, and I think we are well on our way to dealing with it. Regarding the mvn build, I think we can just have it run against the default Python installation for now, which is a bit more risky, but we can mitigate the risk by running CI builds for multiple versions of Python. I also like to test locally, I just spin up a server manually and run `python setup.py test` against multiple versions of Python. This kind of multiple environment testing is made easier using the tool `tox`, maybe we could look at adding this as a requirement. This shouldn't affect the deployment process at all. The only difference is now we can add Python 3 to the package metadata. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #415: Tinkerpop 1448: gremlin-python should be Python 2/3 co...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/415 Sounds good to me. I'll make the 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #415: Tinkerpop 1448: gremlin-python should be Python...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/415 Tinkerpop 1448: gremlin-python should be Python 2/3 compatible https://issues.apache.org/jira/browse/TINKERPOP-1448 This PR adds code to promote Python 2/3 compatibility in the gremlin-python GLV. The main change here involves adding a module, `compat`, that provides 2/3 compatible type info. `compat` defines a class `long` for Python 3. `long` inherits from `int` and behaves like an integer. The advantage of creating this class is it allows application developers to type an integer as `long` in Python 3 code, which will result in serialization as `int64` in GraphSON. `compat` also provides values for standard library `types` module that are not included in Python 3 (`IntType`, `FloatType`, and `LongType`). Finally, this PR uses `six.get_function_code` for introspection, as this API changed with Python 3. While all tests have been updated accordingly, when building with `mvn clean install -pl gremlin-python -DglvPython` tests are only run against the local machine's default version of Python. Therefore, tests are not run on both versions by default, however, they *will* now pass if a users default Python interpreter is Python 3. You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1448 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/415.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 #415 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #408: TINKERPOP-1440: g:Path needs a GraphSON deserializer i...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/408 VOTE +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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #393: Add pytest-runner in order to properly build gremlin-p...
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/393 @spmallette I think when you run the tests in the virtualenv you may have trouble with the site-package test suites being invoked (specifically wheel). To remedy this, I added lib/lib64 to the patterns that should be avoided in test discovery. Because setting the norecursedirs option resets the defaults, I included the standard defaults as well. Hopefully this will avoid further problems. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #393: Add pytest-runner in order to properly build gr...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/393 Add pytest-runner in order to properly build gremlin-python dependencies. Use `pytest-runner` in order to invoke `pytest` using the `setup.py test` command and properly build dependencies. Otherwise, tests will fail unless all dependencies are installed locally. This PR also specifies versions for `tornado` and `aenum`, as well as removing the custom `PyTest` command from `setup.py` (unneeded due to `pytest-runner`). You can merge this pull request into a Git repository by running: $ git pull https://github.com/davebshow/tinkerpop add_pytest_runner Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/393.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 #393 commit 56737a134fc08e56848eed8adc61201bdd0802df Author: davebshow Date: 2016-08-29T19:27:49Z Use pytest-runner to properly build deps on test invocation commit 5c9d9927d20c7f56b9a0bc9c5c1ae0fe5a58f4bf Author: davebshow Date: 2016-08-29T19:33:51Z remove unneeded custom command --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #389: Implemented side effect interface for gremlin-p...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/389 Implemented side effect interface for gremlin-python This is my first go at implementing a side effects interface for gremlin-python. It allows you to do things like: ```python from gremlin_python.structure import Graph from gremlin_python.driver.driver_remote_connection import DriverRemoteConnection rc = DriverRemoteConnection('ws://localhost:8182', 'g') graph = Graph() g = graph.traversal().withRemote(rc) resp = g.V().aggregate('a') side_effects = resp.sideEffects() side_effects.keys() # a list of keys for x in side_effects.get('a'): print(x) ``` Alternately, if the traversal has already been executed, the side effects interface can be accessed like a property: ```python traversal = g.V().aggregate('a') results = traversal.toList() side_effects = traversal.side_effects.get('a') # or call like a function as in previous example: side_effects = traversal.sideEffects().get('a') ``` Now, there may be some cleanup or review necessary, in general I would like to review property getters and setters for the gremlin-python module. Also note, the above examples are run against the 1278 branch with a configuration that includes the GraphSON v2 serializer: ``` - { className: org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerGremlinV2d0, config: { useMapperFromGraph: graph }} # application/vnd.gremlin-v2.0+json ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/davebshow/tinkerpop gremlin_python_side_effects Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/389.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 #389 commit 768c053e9c2b70ee345fdde5ca9ce40d014cac31 Author: davebshow Date: 2016-08-23T20:33:29Z Implemented side effect interface for gremlin-ppython commit 8b0927378ef1f929035b14a0b4340065578ea3fb Author: davebshow Date: 2016-08-23T20:37:53Z removed print statement --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #388: Cleanup of Python code in groovy traversal gene...
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/388 Cleanup of Python code in groovy traversal generator This PR makes some very simple changes to the groovy traversal source generator: - Removes unnecessary `return` statements (Python returns `None` implicitly) - Makes the class `TraversalStrategy` into a real base class. You can merge this pull request into a Git repository by running: $ git pull https://github.com/davebshow/tinkerpop cleanup_python_traversal_generator Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/388.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 #388 commit e1a558dd79f70848df73149dc8a369c7d3ea02b0 Author: davebshow Date: 2016-08-23T16:00:53Z Cleanup of Python code in groovy traversal generator --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #379: added simple WebSocketRemoteConnection
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/379 added simple WebSocketRemoteConnection Also: Made RemoteConnection into actual abstract base class, fixed typo in remote_graph. This PR starts the process of adding a websocket driver to the gremlin-python GLV. It is basically working, except I am not sure how you submit the `Bytecode` object and `traversal_source` to the server, since the `Bytecode` object does not serialize to JSON. As is, the the `RESTRemoteConnection` has the same problem. I'm sure there are just a few things missing of which I am not aware. Also, I am not sure how the `traversal_source` should be passed. My initial thought was to pass it as an alias, but this is obviously not correct. Anyway, let me know how to proceed with this and I will move forward. You can merge this pull request into a Git repository by running: $ git pull https://github.com/davebshow/tinkerpop python_websocket_driver Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/379.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 #379 commit 1f026658f1615d203161fcd469752b6dbb8fc306 Author: davebshow Date: 2016-08-14T22:47:55Z added simple WebSocketRemoteConnection, made RemoteConnection into actual abstract base class, fixed typo in remote_graph --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #343: restructured gremlin-python package
Github user davebshow closed the pull request at: https://github.com/apache/tinkerpop/pull/343 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #343: restructured gremlin-python package
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/343 Sorry, I hurried this because I was leaving town and have not had much access to the internet. Due to the many conflicts it will be simpler to close and create a new PR. I will make sure all tests pass before submitting. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #343: restructured gremlin-python package
Github user davebshow commented on the issue: https://github.com/apache/tinkerpop/pull/343 This would probably be because the way tests are set up the modules are getting executed as a script instead of as a package. I don't know how to fix this off the top of my head--I haven't familiarized myself with how tests are being run, and I unfortunately don't have time to figure it out right now. Ideally, Gremlin-Python would be installed using pip and used normally as a package. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #343: restructured gremlin-python package
GitHub user davebshow opened a pull request: https://github.com/apache/tinkerpop/pull/343 restructured gremlin-python package This PR restructures the Gremlin-Python package to unify packages, modules, and classes under the gremlin_python namespace as discussed on the mailing list. It also provides a minor fix to ensure Python 2/3 compatibility, as well as relative imports for convenience and flexibility. You can merge this pull request into a Git repository by running: $ git pull https://github.com/davebshow/tinkerpop restructure_gremlin_python Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/343.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 #343 commit db111729952c28ccee82d5b9e3aa627b68f2e928 Author: davebshow Date: 2016-06-17T20:56:56Z restructured gremlin-python package --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---