search

mpollmeier / gremlin-scala

[unmaintained] Scala wrapper for Apache TinkerPop 3 Graph DSL
Apache License 2.0
482 stars 76 forks source link

Support .promise for local traversals #235

Open apatzer opened 6 years ago

apatzer commented 6 years ago

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 commented 6 years ago

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.