Repository navigation
make urllib3 an optional dependency? #9267
Description
Activity
#3755, for a bit of background.
IMO, making it optional is worth the breakage. We can raise with an informative error message when we reach that spot and find that urllib3 isn't present.
Reacted by James LambNo strong thoughts on this topic from me. Though alternatively we could replace the
urllib3code with something that only uses the Python standard library. Something likefrom urllib.error import HTTPError from urllib.request import Request, urlopen import time retry_statuses = {429, 502, 503, 504} backoff_factor = 0.2 max_retries = 10 request = Request(url, method="GET") for attempt in range(max_retries + 1): try: with urlopen(request) as response: source = response.read() break except HTTPError as exc: if exc.code not in retry_statuses or attempt == max_retries: raise delay = backoff_factor * (2 ** attempt) time.sleep(delay)
Reacted by James LambI like that suggestion @jrbourbeau
I'd be happy to put up a PR if you all are open to it.
Hey everyone,
I like the stdlib direction here too. Replacing the current urllib3 usage with urllib.request seems like a cleaner approach than introducing optional dependency handling, especially if this is the only direct usage today.If nobody has started working on it yet, I’d be interested in putting together a PR for this.
I will put up a PR.
Reacted by James Bourbeau and Prince KumarI put up #9273, with changes very similar to what @jrbourbeau suggested.
Worth noting that urllib3 remains an optional dependency of
fsspec(throughrequests)Reacted by James Lamb
Description
urllib3is a required dependency ofdistributed:distributed/pyproject.toml
Line 43 in c813e6d
As far as I can tell, the only direct use of it is for
--preload-ing scripts stored at a remote URL (described at https://docs.dask.org/en/stable/customize-initialization.html):distributed/distributed/preloading.py
Lines 132 to 134 in c813e6d
Would you consider making it an optional dependency of
distributed?Benefits of this work
distributedand other projectsdistributedtransitively being the source of findings from CVE scanners (libraries likeurrlib3involved in passing data over networks are popular targets for security researchers)distributedI'll note that none of
distributed's other strong dependencies appear to pull inurllib3, so making it optional here really would have all these benefits for some environments.Acceptance Criteria
distributeddoes not have a hard dependency onurllib3urllib3is installed, and raises an informative error otherwiseApproach
N/A - will defer to maintainers
This would be a breaking change for users who rely on
--preload-ing from a URL, but I think it could be acceptable in exchange for the benefits I listed above.Notes
I'd be happy to put together a PR for this if there is interest.
Thanks for your time and consideration.