Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support .promise for local traversals #235

Open
apatzer opened this issue Feb 28, 2018 · 1 comment
Open

Support .promise for local traversals #235

apatzer opened this issue Feb 28, 2018 · 1 comment

Comments

@apatzer
Copy link

apatzer commented Feb 28, 2018

With an embedded graph DB, if we attempt code like:

graph.V.promise(_.toList)
>> java.lang.IllegalStateException: Only traversals created using withRemote() can be used in an async way

I'm note sure why that would be the case. Even in the local case, we are potentially hitting the disk (slow) so for a performant, non-blocking system (i.e. Scala) one would expect that the IO operations whether network or disk would return a Future[].

It also means that code (like tests) needs to know what type of Graph DB it's connecting to, which feels like it breaks encapsulation.

Do you think .promise() should always return a Future regardless of what it's connecting to?

Happy to do a pull request, but since the error originates in org.apache.tinkerpop.gremlin.process.traversal.Traversal.java:163 the simplistic approach would be to immediately return a Future.successful if endStep is a RemoteStep. And that's unfortunate, as it still means synchronous IO to the underlying DB.

@mpollmeier
Copy link
Owner

I agree, it's annoying to get different behaviour for different DBs.
One option to fix this (and it sounds like that's what you have in mind as well) would be to store the fact that this is a remote connection (either at value or at type level), and then in promise wrap e.g. toList in a Future. You'd have to pass it an execution context then, as well, though.

Another thing that comes to mind: Future is part of the standard library, but there's better implementations for asynchronous computation, e.g. scalaz Task and cats-effect. Just some food for thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants