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

Pip install --dry-run shouln't download full wheels when metadata file available #12603

Open
1 task done
notatallshaw opened this issue Mar 28, 2024 · 8 comments
Open
1 task done
Labels
C: install report pip install --report type: enhancement Improvements to functionality type: performance Commands take too long to run

Comments

@notatallshaw
Copy link
Contributor

Description

When running pip install --dry-run {package} pip downloads the metadata file and then the full wheel

Expected behavior

Dry run installs don't need to download the full wheels

pip version

24.0

Python version

3.11

OS

Linux

How to Reproduce

pip install --dry-run kaleido==0.2.1

Output

$ pip install --dry-run kaleido==0.2.1
Collecting kaleido==0.2.1
  Downloading kaleido-0.2.1-py2.py3-none-manylinux1_x86_64.whl.metadata (15 kB)
Downloading kaleido-0.2.1-py2.py3-none-manylinux1_x86_64.whl (79.9 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 79.9/79.9 MB 34.6 MB/s eta 0:00:00
Would install kaleido-0.2.1

Code of Conduct

@notatallshaw notatallshaw added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Mar 28, 2024
@pfmoore
Copy link
Member

pfmoore commented Mar 28, 2024

PRs welcome. However, one potential upside of downloading the wheels is that if the dry run is followed by an actual install, the install itself will be faster as everything needed will be cached. So it's not immediately obvious that this change would be beneficial overall.

@notatallshaw
Copy link
Contributor Author

notatallshaw commented Mar 28, 2024

PRs welcome. However, one potential upside of downloading the wheels is that if the dry run is followed by an actual install, the install itself will be faster as everything needed will be cached. So it's not immediately obvious that this change would be beneficial overall.

I think there are two main use cases for dry run, one where you're validating what would install and then installing, and one where you're collecting the output of dry run as part of a larger environment management process, in this case you may not be installing locally, you could be preparing files for a docker build, or many other things.

I think you're thinking of the first case. Even in that case you may decide that the versions are wrong and you want to update the requirements, you may need to do this several times. If the wheels are very large, it's a lot of wasted download and time as you iterate finding the right requirements.

@rootsmusic
Copy link

rootsmusic commented Apr 11, 2024

I've inserted the --dry-run option, but it seems to be ignored by pip. Is that unexpected behavior related to this issue?

@notatallshaw
Copy link
Contributor Author

notatallshaw commented Apr 11, 2024

I've inserted the --dry-run option, but it seems to be ignored by pip. Is that unexpected behavior related to this issue?

Dry run works fine for me, in the sense that it doesn't install any packages, if you have an issue that appears to be a bug create a new issue with steps to reproduce, including your environment such as your pip version etc.

@rootsmusic
Copy link

Dry run works fine for me

@notatallshaw, you're right. I mistakenly assumed that it wasn't, because the output says "Downloading" (which I quickly interrupted). At the end, the output does say "would install" the downloaded package.

@notatallshaw
Copy link
Contributor Author

notatallshaw commented Apr 13, 2024

I started working on a PR and I think it's going to be a smaller change once #12300 lands because there's some specific legacy version and specifier warnings which assume the full wheel file is there. So I'm going to wait until that is no longer a concern rather than trying to fix that code.

Although this is going to be bigger than a 10 line change, because InstallRequirement needs to be changed so it can use the metadata file if it is there, instead of the wheel file. This seems like a positive change though, should be less IO.

@pradyunsg pradyunsg removed the type: bug A confirmed bug or unintended behavior label Apr 14, 2024
@pradyunsg
Copy link
Member

x-ref #12186

@notatallshaw
Copy link
Contributor Author

That PR seems effectively dead, hopefully I can touch this code in a much simpler manner.

@ichard26 ichard26 added type: enhancement Improvements to functionality C: install report pip install --report type: performance Commands take too long to run and removed S: needs triage Issues/PRs that need to be triaged labels Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: install report pip install --report type: enhancement Improvements to functionality type: performance Commands take too long to run
Projects
None yet
Development

No branches or pull requests

5 participants