Drop keepalive from requirements.txt (Closes: #846871) #79
Conversation
As keepalive support is a weak dependency (absence makes keepalive enabling only raise a warning and otherwise a no-op), listing it as a dependency causes trouble with test setups. Demoting it to an extra (feature) dependency.
|
history: #70 ... keepalive is used in rdflib's SPARQLStore... https://github.com/RDFLib/rdflib/blob/master/rdflib/plugins/stores/sparqlstore.py#L248 i'm not sure if dropping this is wise, but i can see how debian doesn't want to make an own package for it... @wikier maybe we should either internalize keepalive to SPARQLWrapper or just use some lib like requests / urllib3? |
|
@joernhees, is that actually the right history / context? i was rather thinking of the "optional dependencies" comment in #37 as context. this is not about the keepalive functionality (with it absent, tests run through still), but solely about whether it is declared in setup.py as an installation requirement or an extra requirement. afaict, the only ill-effect this can have is that if an application previously required "sparqlwrapper" and expects keepalive functionality to actually work, they will now need to require "sparqlwrapper[keepalive]". (and thus, things that don't need keepalive can have their setup.py dependencies satisfied even in absence of the keepalive package). as far as debian packaging is concerned, there is no keepalive package right now, but even if there were, unless sparqlwrapper declares it actually depends on keepalive for its operation, it wouldn't be installed in the abovementioned situation. in other words: there is a mismatch between how setup.py declares the dependencies ("keepalive is required"), and how debian declares it ("keepalive is recommended"). we (debian packagers) could do the "easy" thing and follow setup.py's declaration by sparqlwrapper declare a hard dependency on keepalive, but my opinion is that it wouldn't reflect the actual dependency, and thus setup.py's "install_requires" doesn't either. using requests / urllib3 would probably solve this issue too (but seems to have been on the roadmap for some time). internalization would not be helpful from the packaging point of view. current status in packaging is that i'm applying the pullrequested patch in debian, taking the risk that programs rely on sparqlwrapper to pull in keepalive implicitly but actually getting no-op there. |
|
Here my two cents to the discussion from the point of view of SPARQLWrapper:
|
As keepalive support is a weak dependency (absence makes keepalive enabling only raise a warning and otherwise a no-op), listing it as a dependency causes trouble with test setups. Demoting it to an extra (feature) dependency.
The hard dependency statement has caused trouble with test setups where the contentually unnecessary keepalive package caused a dependency failure. Please consider classifying keepalive as a feature dependency as above or similar.