justindujardin / pathy

simple, flexible, offline capable, cloud storage with a Python path-like interface
Apache License 2.0
171 stars 23 forks source link

fix: upgrade smart-open range to >=2.2.0,<4.0.0 #36

Closed tiangolo closed 3 years ago

tiangolo commented 3 years ago

⬆️ Upgrade smart-open pin, to fix botocore requiring urllib3 < 1.26

When standard pip installs the latest pathy, it installs smart-open < 3.0.0, and it requires botocore, which in turn requires urllib3 < 1.26. But it seems something else also requires urllib3, and that one is resolved first, so pip (the current version) installs the newer urllib3 and shows a warning about the conflicting dependency.

But when using Pex, it doesn't show a warning, it fails right away.

The funny thing is, pip, with the new resolver (with --use-feature=2020-resolver) can detect it, then it uninstalls the first resolved urllib3 and installs the older urllib3 1.25.x. But there's no way to force Pex to use that new resolver :pensive:

The new version of smart-open 3.0.0 doesn't require botocore, instead it has new optional requirements:

pip install smart_open[s3]

or

pip install smart_open[all]

This PR only increases the pin range for smart-open.

There would also be needed a Pathy version bump and release :grimacing: :sweat_smile:

And I don't know if it would make sense to make it require smart-open[gcp] directly, but that would couple Pathy to Google Cloud Storage. Or maybe it could make sense to add optional dependencies for Pathy similar to smart-open, so pathy[gcp] would include smart-open[gcp].

codecov[bot] commented 3 years ago

Codecov Report

Merging #36 (9150aa2) into master (18f4021) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   80.41%   80.41%           
=======================================
  Files           7        7           
  Lines         878      878           
=======================================
  Hits          706      706           
  Misses        172      172           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 18f4021...9150aa2. Read the comment docs.

justindujardin commented 3 years ago

:tada: This PR is included in version 0.3.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

justindujardin commented 3 years ago

And I don't know if it would make sense to make it require smart-open[gcp] directly, but that would couple Pathy to Google Cloud Storage. Or maybe it could make sense to add optional dependencies for Pathy similar to smart-open, so pathy[gcp] would include smart-open[gcp].

Pathy already has a "gcs" package extra that includes the GCS library dep, but it could be interesting to add "smart_open[gcs]" to our "gcs" dependencies. I wonder if having "smart_open" as a required dep and then "smart_open[gcs]" as an extra would conflict?