Skip to content
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

Overly strict Python dependency requirements #30

Closed
gitpushdashf opened this issue Dec 17, 2020 · 11 comments
Closed

Overly strict Python dependency requirements #30

gitpushdashf opened this issue Dec 17, 2020 · 11 comments

Comments

@gitpushdashf
Copy link

I'm getting this warning (even though it looks like an error) about the new PIP dependency resolver.

ERROR: After October 2020 you may experience errors when installing or updating packages. This is because pip will change the way that it resolves dependency conflicts.

We recommend you use --use-feature=2020-resolver to test your packages with the new resolver before it becomes the default.

google-fhir 0.6.2 requires protobuf==3.13.0, but you'll have protobuf 3.14.0 which is incompatible.

I can see that the requirements are set exactly: https://github.com/google/fhir/blob/master/py/requirements.txt For a library, would it be better to do something like this?

absl-py >= 0.10.0 < 0.11.0
backports.zoneinfo >= 0.2.1 < 0.3.0
mypy-protobuf >= 1.23 < 2
protobuf >= 3.13.0 < 4
python-dateutil >= 2.8.1 < 3
six >= 1.15.0 < 2

(I haven't tested those exactly, it's just speculation and assuming those are all semver.)

Thank you!

@Cam2337
Copy link
Collaborator

Cam2337 commented Dec 18, 2020

Most of this stems from the bazel-compatibility story. In particular, we want to be sure that the packages installed and leveraged when bazel (build/test/run)ing are identical to what you get via pip install. However, bazel+pip interoperability (as of rules_python 0.0.2) presents some difficulties here w.r.t. transitive dependency resolution (e.g. see #125, #102).

Because of this, at the moment we are opting for explicit versions to promote determinism and safety. I've been experimenting with the new rules_python release (0.1.0) which should resolve the transitive dependency issues along with a few other bugs related to bazel+pip, but it introduces some changes into how Python3 implicit namespace packages are being handled (converting implicit namespace packages to pkg-util namespace packages). Once I can ensure this is working safely+properly we'll try relaxing these constraints to something more permissible!

@gitpushdashf
Copy link
Author

Ah, I gotcha. That makes sense. Thanks for explaining. Bazel seems tricky to work with.

@gitpushdashf
Copy link
Author

gitpushdashf commented Feb 25, 2021

Hey! Have you had any luck on this by chance? We're trying to use mypy-protobuf 2.4 which should give grpc stub generation for stricter type checking.

If not, would it be possible to bump the explicit dependencies to the current latest versions?

Thank you!

Edit: Does look like the version is getting installed anyway, so this may not hinder us after all.

@Cam2337
Copy link
Collaborator

Cam2337 commented Mar 6, 2021

Hey @gitpushdashf -- I spent some time looking at this a couple of days ago.

With regards to mypy-protobuf, we only actually leverage the plugin when building the distribution from source and packaging the resulting stubs. As such, I don't think this really belongs in requirements.txt (see here). I've made a change to move this out of requirements.txt and into build_distribution.sh. In the meantime, it shouldn't be a problem to install your desired version (and it seems like from your comment this worked out!).

On a related note, I spent time verifying rules_python 0.1.0 compatibility with our package namespace structure. It seems to be handling the package namespaces correctly for bazel, so I created a change to adopt this, and subsequently move requirements.txt to:

absl-py~=0.10.0
backports.zoneinfo~=0.2.1
protobuf~=3.13.0
python-dateutil~=2.8.1

Which omits the transitive deps of these packages and allows for compatible versions to be installed.

Both of these changes are under review.

@gitpushdashf
Copy link
Author

Ah, that's fantastic! Thank you. I didn't know about the ~= in requirements.txt.

@Cam2337 Cam2337 closed this as completed in a13c306 May 6, 2021
@gitpushdashf
Copy link
Author

Hey!

Unfortunately, looks like this isn't working quite as expected. The patch level looks to be adjustable, but that's all.

I seem to get this with Pipenv or Poetry.

package $ poetry update
Updating dependencies
Resolving dependencies... (11.7s)

  SolverProblemError

  Because google-fhir (0.7.2) depends on protobuf (>=3.13.0,<3.14.0)
   and no versions of google-fhir match >0.7.2,<0.8.0, google-fhir (>=0.7.2,<0.8.0) requires protobuf (>=3.13.0,<3.14.0).
  So, because package depends on both protobuf (^3.17.3) and google-fhir (^0.7.2), version solving failed.

  at /usr/local/lib/python3.8/site-packages/poetry/puzzle/solver.py:241 in _solve
      237│             packages = result.packages
      238│         except OverrideNeeded as e:
      239│             return self.solve_in_compatibility_mode(e.overrides, use_latest=use_latest)
      240│         except SolveFailure as e:
    → 241│             raise SolverProblemError(e)
      242│
      243│         results = dict(
      244│             depth_first_search(
      245│                 PackageNode(self._package, packages), aggregate_package_nodes

I wonder if protobuf =< 3.13.0 > 4 would work.

@Cam2337
Copy link
Collaborator

Cam2337 commented Jul 26, 2021

Hi @gitpushdashf 👋,

Apologies, I should have been more explicit that a13c306 was intended to only allow fluctuations in patch version.

After further review, for mature packages we should be fine to also free this up to minor/patch version, which should fix your dependency resolution problem.

E.g. requirements.txt now becomes:

absl-py~=0.10.0
backports.zoneinfo~=0.2.1;python_version<"3.9"
protobuf~=3.13
python-dateutil~=2.8

See PEP0440 for more details on ~=.

This is in review.

@gitpushdashf
Copy link
Author

Ohh, that is interesting. I didn't realize that ~=X.Y.Z behaved so differently from ~=X.Y.

This sounds great. We're curious to try the newer protobuf version in particular, since 3.13.0 has been out for a while.

Thank you!

@gitpushdashf
Copy link
Author

Hey, I see that you guys cut 0.7.3 but haven't seen it on pypi yet. Is it coming soon?

Thank you!

@Cam2337
Copy link
Collaborator

Cam2337 commented Aug 10, 2021

Hey! 👋

A few more changes need to go in before this is tested and cut (0.7.3 has not been cut yet, though some of the changes around further loosening Python dependencies are pushed to GitHub).

If this is time-sensitive, you can clone+build the package artifacts from source to test out the pypi package.

@gitpushdashf
Copy link
Author

Ah, I understand. I just saw the 0.7.3 in the Python sources, I guess.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants