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

Default to --no-reuse-hashes #1184

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

graingert
Copy link
Member

A breaking change for v6

Changelog-friendly one-liner: default to --no-reuse-hashes

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

Now that hashing is way faster than it once was, I think the slight speed improvement from caching no longer outweighs the danger of trusting local/accidental changes to the .txt pins

@codecov
Copy link

codecov bot commented Jul 20, 2020

Codecov Report

Merging #1184 (5decf31) into master (6ed9301) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1184   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files          34       34           
  Lines        3042     3042           
  Branches      327      327           
=======================================
  Hits         3032     3032           
  Misses          5        5           
  Partials        5        5           
Impacted Files Coverage Δ
piptools/scripts/compile.py 100.00% <ø> (ø)

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 6ed9301...5decf31. Read the comment docs.

@atugushev atugushev added the backwards incompatible Backwards incompatible change label Jul 27, 2020
@atugushev atugushev changed the title default to --no-reuse-hashes Default to --no-reuse-hashes Jul 28, 2020
@atugushev atugushev added this to the 6.0.0 milestone Nov 21, 2020
@jdufresne
Copy link
Member

Now that hashing is way faster than it once was, I think the slight speed improvement from caching no longer outweighs the danger of trusting local/accidental changes to the .txt pins

If this is the case, would it make sense to deprecate/remove the feature entirely?

@graingert
Copy link
Member Author

@jdufresne I'm in favour of that

@graingert
Copy link
Member Author

@jdufresne #1135 would probably need doing first though

@atugushev atugushev removed this from the 6.0.0 milestone Mar 11, 2021
@atugushev
Copy link
Member

FTR: I've removed this PR from 6.0.0. I'm going to prepare a release soon and it's unclear to me whether there should be changed the default value for --no-reuse-hashes or should be completely removed. Seems like this PR could be postponed for now.

@plannigan
Copy link
Contributor

Artifactory doesn't support the /json endpoint, so users in a corporate environment that don't directly pull from PyPI would be required to always download each file to be hashed. So removing the capability to reuse hashes would increase the time it takes to compile a lock file. This is part of the reason I implemented single hash (#1406).

@graingert
Copy link
Member Author

@plannigan #1135 we could pull the hashes from the simple API

@plannigan
Copy link
Contributor

Good to know. If sequence was try /json/try simple/download file then that would resolve my concerns.

@graingert
Copy link
Member Author

Probably not worth doing the /json endpoint now as the simple api will already have been downloaded with the hashes in

@neykov
Copy link
Contributor

neykov commented Nov 13, 2022

There are widely used Python projects that rely extensively on --find-links / --extra-index-url for distribution that don't provide checksums in their index files. For example:

pytorch is a ~2GB download. Until these projects catch up, generating the hashes will be too expensive (slow and bandwidth intensive) if --reuse-hashes is dropped.

@ssbarnea ssbarnea added this to the 7.0.0 milestone Dec 14, 2022
@atugushev atugushev removed this from the 7.0.0 milestone Jul 14, 2023
@atugushev atugushev marked this pull request as draft July 28, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible Backwards incompatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants