[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/478 > the type of future returned will depend on the underlying RemoteConnection implementation. That depends on whether you are okay leaking remote connection implementation through this API. I have been trying to think about this traversal API as something distinct from transport (in which case you would probably want an abstraction). --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/478 Coupling the traversal API to a web framework does not seem appealing to me. Could we consider using the [futures backport](https://pypi.python.org/pypi/futures), which backports the Python 3 standard library features to Python 2? --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 Pushed the split names. I didn't see issues related to these changes running `process-docs.sh`. May have been related to the previous removal of GraphSONWriter/Reader. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 Maybe we should clarify what APIs really need to be preserved. I have been thinking about the `process` package, and `structure` module as the Gremlin API, and the `driver` and `io` as distinct integrations. I'm working on splitting reader/writer now, but I'm not convinced this API is worth replicating here. We can chalk it up to my ignorance of the greater ecosystem machinery. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 Thanks. I see the code there. What is not clear to me is why these two things are divided, and why that API should be replicated here. I'm having a hard time doing this change because it makes it more cumbersome to use (at least for the integration I'm considering). I can vaguely imagine using readers and writers in a migration scenario where input and output are different. Can you help me understand the use case in the context of this GLV, where client IO uses a single format? --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 - rebased - dropped `unittest.TestCase` assertions - added custom graphson_io to DriverRemoteConnection I haven't had a chance to get back to the GraphSONIO vs Reader/Writer question. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 (also, ACK on rebase -- I should be getting to it 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 #448: Python glv graphson update
Github user aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 > you changed GraphSONWriter and GraphSONWriter to GraphSONIO This was not Python-specific, but felt like a natural design to me. I unified the I/O per type, and tied those together at the top level in `GraphSONIO`. It seems strange to me to have the two sides of the same encoding split between classes. I thought I saw something in the java impl called `GraphSONIO`, but maybe I was mistaken. I can split them up, but then it means carrying around two things to encode/decode everywhere, instead of one: `connection = DriverConnection(..., graphson_reader=GraphSONReader(...), graphson_writer=GraphSONWriter(...))` vs. `connection = DriverConnection(..., graphson_io=GraphSONIO(...))` I'll study the Java impl. a little closer to see if I can understand the benefit. Please let me know if you have any further thoughts. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 argh. needs a third rebase, too :-/ --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 > graphson_io=None to the __init__ method signature I think that is a fine idea. I was mostly focused on the GraphSON API and not concerned with adding it to the token RemoteConnection implementation (figuring it would get pulled in if someone saw cause to do it). If you think it's worth doing now (and it won't invalidate this vote), I can push the update I have here. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 I'm ambivalent on that. I just noticed unittest was present, and the bare assertions were not very informative. I can take that commit out if you think it's not worth it now. --- 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 aholmberg commented on the issue: https://github.com/apache/tinkerpop/pull/448 Thanks Marko. Rebased again. Also removed print statement from one test and updated Python tests to use `unittest.TestCase` assertions. --- 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 #448: Python glv graphson update
GitHub user aholmberg opened a pull request: https://github.com/apache/tinkerpop/pull/448 Python glv graphson update I have a series of gremlinpython graphson serdes simplifications, followed by a refactor to make type mappings an instance variable, rather than a module-level mapping. I did not attempt to preserve the module API since `gremlinpython` has not entered GA release status. I welcome any feedback. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aholmberg/tinkerpop python-glv-graphson-update Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/448.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 #448 commit 35f11792254235c9414d8297ef9d67fbad1076ee Author: Adam Holmberg Date: 2016-09-23T19:09:03Z gremlinpython: simplify graphson serdes functions commit c461e13f2bf16cffe19daa50212f70ff50634f1e Author: Adam Holmberg Date: 2016-09-23T19:38:23Z gremlinpython: remove keyword shadows in graphson commit ff876a0c612ed84d1d4013c38209c48efe9c6dfa Author: Adam Holmberg Date: 2016-09-23T20:04:04Z gremlinpython: more simplification of graphson routines commit 91c5830041dae556ef823cfa6584f1e22d2b7179 Author: Adam Holmberg Date: 2016-09-30T18:22:37Z simplification in graphson io commit 20226eff3ee4f9eefef3dc91f2d516ed47af0830 Author: Adam Holmberg Date: 2016-10-04T18:56:21Z gremlinpython: refactor GraphsonIO to use instance type maps --- 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 aholmberg commented on the pull request: https://github.com/apache/tinkerpop/commit/6ed7edc0b4ad2abf933e917812d49ad92230c8d1#commitcomment-18474370 In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy: In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy on line 101: (Please let me know if I can be of assistance on the Python front.) --- 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 aholmberg commented on the pull request: https://github.com/apache/tinkerpop/commit/6ed7edc0b4ad2abf933e917812d49ad92230c8d1#commitcomment-18474349 In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy: In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy on line 101: Also wondering if we are planning to convert method names to lower/underscore to match the target language conventions, as described [here](http://tinkerpop.apache.org/docs/3.2.1-SNAPSHOT/tutorials/gremlin-language-variants/#gremlin-language-variant-conventions). --- 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 aholmberg commented on the pull request: https://github.com/apache/tinkerpop/commit/6ed7edc0b4ad2abf933e917812d49ad92230c8d1#commitcomment-18473940 In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy: In gremlin-variant/src/main/groovy/org/apache/tinkerpop/gremlin/python/GremlinPythonGenerator.groovy on line 95: May I suggest that we use a trailing, instead of leading underscore for keyword collisions? This would make it more natural to begin typing for completion. It would also adhere to Python style guidelines, which says leading underscore indicates private, internal use, and to use trailing underscores to avoid keyword collisions. https://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles --- 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 #364: tweaks in gremlin language variants doc
GitHub user aholmberg opened a pull request: https://github.com/apache/tinkerpop/pull/364 tweaks in gremlin language variants doc I noticed a few possible errors while reading the "Gremlin Language Variants" tutorial. You can merge this pull request into a Git repository by running: $ git pull https://github.com/aholmberg/tinkerpop lang-var-tutorial-tweak Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/364.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 #364 commit a3f5dab17c9bcfbb464f034608b5a89fa57eed55 Author: Adam Holmberg Date: 2016-07-27T17:57:22Z tweaks in gremlin language variants doc --- 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. ---
