feat(pypi/parse_requirements): get dists by version when no hash provied #2695
Conversation
…ided Modify _add_dists to fetch files by version if no sha256 hashes are available, and add corresponding unit tests. - Updated _add_dists to check for the presence of hashes before fetching by hash. - Added an `else` condition that iterates through index URLs to find wheels and source distributions matching the requirement's version when no hashes are provided. - Added unit tests in //tests/pypi/parse_requirements to verify version-based fetching.
There was a problem hiding this comment.
Thanks for the PR. For this to be more robust and performant, it would be great to add an extra field that is the result of SimpleAPI parsing where we store sha256s by version of the package in the index.
The code is here:
https://github.com/bazel-contrib/rules_python/blob/main/python/private/pypi/parse_simpleapi_html.bzl#L96
|
OK, I have updated the implementation based on my own feedback and updated the |
I have became a co-author of the PR so I'll ask others to review.
Thank you for your help. It's my first time doing this. I mistakenly closed it but reopened it. |
Thanks for the updates and feedback! I’m still new to this, so I really appreciate your help. I’ve checked the updated implementation and docs, and I’ve learned a lot. I tested it in my environment, and it works great. |
Whilst integrating bazel-contrib#2695 I introduced a regression and here I add a test for that and fix it. The code that was getting the filename from the URL was too eager and would break if there was a git ref as noted in the test. Before this commit and bazel-contrib#2695 the code was not handling all of the cases that are tested now either, so I think now we are in a good place. I am not sure how we should handle the `git_repository` URLs. Maybe having `http_archive` and `git_repository` usage would be nice, but I am not sure how we can introduce it at the moment. Work towards bazel-contrib#2363
Whilst integrating #2695 I introduced a regression and here I add a test for that and fix it. The code that was getting the filename from the URL was too eager and would break if there was a git ref as noted in the test. Before this commit and #2695 the code was not handling all of the cases that are tested now either, so I think now we are in a good place. I am not sure how we should handle the `git_repository` URLs. Maybe having `http_archive` and `git_repository` usage would be nice, but I am not sure how we can introduce it at the moment. Work towards #2363
This pull request modifies the SimpleAPI HTML parsing to add a new
field where we can get the
sha256values by package version. Thisallows us to very easily fallback to all packages of a particular
version when using
experimental_index_urlif the hashes are notspecified.
The code deciding which packages to query the SimpleAPI for has been
also modified to only omit queries for packages that are included via
direct URL references.
If we fail to get the data from the SimpleAPI, we will fallback to
pipand try to install it via the legacy behaviour.Fixes #2023
Work towards #260
Work towards #1357
Work towards #2363