[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-12-16 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/478 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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-12-08 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-12-07 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 Rebase is complete now - tests are all good with: ```text $ mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false -DincludeNeo4j ``` --- If your

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-12-07 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 I'm rebasing on `tp32` to clean up that conflict. this branch was pretty far behind. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-12-07 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-29 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-29 Thread aboudreault
Github user aboudreault commented on the issue: https://github.com/apache/tinkerpop/pull/478 Except my initial concern, can you confirm we can use the latest way with all Python versions and with/without a custom driver? It would be nice to have an unified way that just works

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-29 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 It's been about a week since @davebshow laid out the approach he's recommending here. I assume the silence means that there are no objections to that approach and we have consensus on what to

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-20 Thread davebshow
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:

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-16 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-16 Thread newkek
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/478 I think the `promise()` method is more elegant as well, as it avoids adding many new methods in the Traversal API --- If your project is set up for it, you can reply to this email and have your

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-16 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 I think I agree with @okram for now that we not add a lot of methods just yet. It's easy to add methods, but harder to take them away later. Let's be sure this gets used in the fashion we

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-14 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 I ran about 50 million messages through Gremlin Server over the weekend and didn't see any problems as a result of my changes here. I think this seems pretty solid now. I'd quietly asked both

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-11 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-10 Thread aholmberg
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-10 Thread aholmberg
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-10 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 thanks @davebshow - i might ask for your assistance on implementing that. still trying to get it right for java. that blocking call in `RemoteStep` stinks. having a bit of a hard

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-10 Thread davebshow
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

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-03 Thread newkek
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/478 The issue with using a secondary thread pool that will start a new thread for each Traversal execution would be more important when the Traversal is backed by a RemoteConnection. Where each

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-03 Thread jorgebay
Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/478 > What makes `toListAsync()` more "fully async" compared to `promise(traversal::toList)`? Internally, from a Java perspective anyway, `toListAsync()` does the same thing, doesn't it? By

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-02 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 > CoreTraversalTest is that it uses mutating steps and thus, not subject to OLAP testing Ah - didn't think to check if that was fully ignored. I'll fix that up. --- If your project

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-02 Thread spmallette
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/478 > Why do you ignore promise tests for GroovyTranslator and JavaTranslator. Seems we would want that to work as well. I thought I was getting errors because of the lambda i use to

[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal

2016-11-02 Thread okram
Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/478 Wow. This is cool. Questions/comments: 1. Why do you ignore promise tests for `GroovyTranslator` and `JavaTranslator`. Seems we would want that to work as well. 2. Why not have a