-
Notifications
You must be signed in to change notification settings - Fork 72
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
move SparqlHttpModule to another project #332
Comments
Maatary
added a commit
to Maatary/banana-rdf
that referenced
this issue
Oct 31, 2017
Maatary
added a commit
to Maatary/banana-rdf
that referenced
this issue
Oct 31, 2017
Maatary
added a commit
to Maatary/banana-rdf
that referenced
this issue
Oct 31, 2017
Added deprecation marker based on discussion in issue banana-rdf#332
Maatary
added a commit
to Maatary/banana-rdf
that referenced
this issue
Oct 31, 2017
Maatary
added a commit
to Maatary/banana-rdf
that referenced
this issue
Oct 31, 2017
Added deprecation marker based on discussion in issue banana-rdf#332
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The SparqlHttpModule is only used by Jena, and is a blocking call over HTTP which is pretty much an anti-pattern in a reactive world.
The Sparql Engine implemented here is:
Which means that any code calling this will block until the result comes back.
If the underlying code were able to work through callbacks, allowing a few threads to do the fetching and parsing of results, that would allow the above api to be Future based, which would be a very big improvement.
But even then there would be additional things needed for a good cross-implementation api such as: unified error classes, etc... all of which indicates that such a function should really belong in its own package.
So even though this mirrors the way Jena does things, it looks bad in banana-rdf. Given that this is only implemented by the Jena implementation it can also be confusing...
So it is ok to have this here as a reminder that something better needs to be done. Therefore I recommend that these classes and vals be deprecated with a comment linking to this issue, as that may stir people to come up with better answers, and also be prepared for this to be removed in the longer term.
The text was updated successfully, but these errors were encountered: