add size to pylock.toml #13395
add size to pylock.toml #13395
Conversation
pre-commit.ci autofix
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
| try: | ||
| session = PipSession() | ||
| response = session.head(self.url, allow_redirects=True) | ||
| response.raise_for_status() |
There was a problem hiding this comment.
This probably should be handled. Does ValueError cover this? It is not obvious.
| @@ -508,6 +508,23 @@ def hash_name(self) -> str | None: | |||
| def show_url(self) -> str: | |||
| return posixpath.basename(self._url.split("#", 1)[0].split("?", 1)[0]) | |||
|
|
|||
| @property | |||
| def size(self) -> int | None: | |||
There was a problem hiding this comment.
I'm not convinced we should do additional network access to get the sizes.
I would rather envision an approach where we add a size attribute to InstallRequirement.download_info (as in a subclass of DirectUrl, or another class that would be more specialized to record the provenance URL, PEP710-style), and populate it at the same time as we currently populate download_info. This also means storing the original size in the cache (origin.json) next to the hashes.
It's more complex but like more efficient and robust.
That's just an intuition though, as I have not investigated deeply.
There was a problem hiding this comment.
it's a patch over an "experimental" feature. you need the "complex" and "efficient" version at this point ?
There was a problem hiding this comment.
My understanding is the term experimental means here is that pip may change the CLI or semantics of the feature between releases. Not that the solution isn't expected to be robust.
I have the same concern here, particularly for the use case of locking a large existing environment, doing hundreds of extra network requests may cause the lock to be significantly slower or fail.
There was a problem hiding this comment.
Another reason is that it would make sense for file sizes to follow the same flow as hashes, as they are closely related. I believe the could be obtained and verified at the same places in the codebase. It will likely make things easier to follow and understand.
There was a problem hiding this comment.
OK, I agree, but that is a bigger change, will need time. Competitive PR is welcomed
to resolve #13393