Stop pinning urllib3 (only done for testing dep) #69
Conversation
|
Thanks for the review and for triggering CI, @andreasprlic - I can take a look at the failing unit tests and try to sort it out soon. |
|
fwiw, behavior looks correct to me when I test manually -- I wonder if the cassettes need to be regenerated? |
This fixes a problem where one cannot install `bioutils` into an environment where `urllib3` version 2 is being used. In other words, merely installing `bioutils` forces you to use `urllib3` version 1. There's a positive side effect too where installing `bioutils` no longer pulls in an unneeded dependency. Why are we pinning an unused dependency? ======================================== Previously, `vcrpy` (a test dependency) would not support urllib3 version 2. Tests would fail to run because of an attempted import of `urllib3.connectionpool.VerifiedHTTPSConnection` (not present in v2): biocommons#46 (comment) To resolve the error, `urllib3` was pinned to version `1.26.*`: biocommons#49 This definitely worked, but `urllib3` is *not* directly used by the package, only indirectly used during testing. Why pin a transitive test-only dependency? ========================================== Unfortunately, the way that `urllib3` was pinned doesn't just affect CI for this repository, but affects *any* environment in which `bioutils` is installed. Specifically, `urllib3` will *always* be installed as a dependency (or possibly downgraded from v2) when you `pip install bioutils`! There are ways we could have pinned `urllib3` only in CI without affecting package dependencies, but that's not important anymore. Is the original problem solved? =============================== Yes! `vcrpy` version 4.3.1 (released in May of 2023) started supporting `urllib3` version 2! > Support urllib3 v1 and v2. NOTE: there is an issue running urllib3 v2 > on Python older than 3.10, so this is currently blocked in the > requirements. Hopefully we can resolve this situation in the future. > Thanks to shifqu, hartwork, jairhenrique, pquentin, and vEpiphyte for > your work on this. https://vcrpy.readthedocs.io/en/latest/changelog.html
DavidCain
force-pushed
the
dcain-unpin-urllib3
branch
from
a2773d3
to
6f3e4d4
Compare
|
Thank you! @andreasprlic - tests do seem to be passing now. |
|
Somehow I am jinxing this. The tests fail in the CI. Should we try regenerating the cassettes, as @jsstevenson suggested? |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
|
Removing |
Thank you, @ahwagner -- I'll confess that I have no idea what "cassettes" even mean in this context so I will gladly take assistance! |
|
This should be good to go now, and is ready for re-review. I also added in a fix to the makefile to build virtual env properly. |
|
Thank you all so much! Any idea when we might be able to get a new release of |
|
doing a release should be easy (just placing a git tag). Let me try that. |
This fixes a problem where one cannot install
bioutilsinto anenvironment where
urllib3version 2 is being used. In other words,merely installing
bioutilsforces you to useurllib3version 1.There's a positive side effect too where installing
bioutilsno longerpulls in an unneeded dependency.
Why are we pinning an unused dependency?
Previously,
vcrpy(a test dependency) would not support urllib3version 2. Tests would fail to run because of an attempted import of
urllib3.connectionpool.VerifiedHTTPSConnection(not present in v2):#46 (comment)
To resolve the error,
urllib3was pinned to version1.26.*:#49
This definitely worked, but
urllib3is not directly used by thepackage, only indirectly used during testing.
Why pin a transitive test-only dependency?
Unfortunately, the way that
urllib3was pinned doesn't just affect CIfor this repository, but affects any environment in which
bioutilsis installed. Specifically,
urllib3will always be installed as adependency (or possibly downgraded from v2) when you
pip install bioutils!There are ways we could have pinned
urllib3only in CI withoutaffecting package dependencies, but that's not important anymore.
Is the original problem solved?
Yes!
vcrpyversion 4.3.1 (released in May of 2023) started supportingurllib3version 2!https://vcrpy.readthedocs.io/en/latest/changelog.html