Skip to content

Upgrade pip inside of any newly created pyenv#5195

Merged
pavera merged 3 commits intomainfrom
pavera/update-pip-in-pyenv
May 31, 2022
Merged

Upgrade pip inside of any newly created pyenv#5195
pavera merged 3 commits intomainfrom
pavera/update-pip-in-pyenv

Conversation

@pavera
Copy link
Copy Markdown
Contributor

@pavera pavera commented May 25, 2022

As part of #5024 we identified an issue where newly created python versions installed via pyenv were not installing pre-compiled wheels but attempting to compile during pip installs.

This PR fixes the issue by updating pip before we attempt to install the python helper requirements.txt into a new pyenv as a newly installed pyenv will have a default pip version (python 3.6.9 has pip 18.0.1 for example).

This should speed up CI/tests, and will allow the above referenced work with earthly to move forward.

@pavera pavera requested a review from a team as a code owner May 25, 2022 21:24
Copy link
Copy Markdown
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wheel downloading not implemented until after pip version 18?

@pavera
Copy link
Copy Markdown
Contributor Author

pavera commented May 26, 2022

yeah, looks like pip 19 is what added support for the manylinux2010 wheels...
https://pip.pypa.io/en/stable/news/#v19-0

Copy link
Copy Markdown
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a thought...

python 3.6 was EOL'd 5 months ago... Does python 3.7 include a new enough version of pip that we wouldn't need this workaround?

If so, rather than adding this, what if we dropped support for python 3.6 and kept our lives simple? I'm not sure what the process is for dropping support for a particular python version, but if we don't have one, it's something we'll need to standardize anyway.

Or do we anticipate that even beyond this particular issue, that we'll still want to always update pip inside of new pyenv's?

To be clear, I'm not against this PR, especially if we need it to unblock the earthly work. Just thought I'd check if we can fix forward rather than workaround.

pip>=21.3.1 # Allow earlier versions to retain python 3.6 support
pip-tools>=6.4.0 # Allow earlier versions to retain python 3.6 support
pip>=21.3.1,<=22.1.1 # Allow earlier versions to retain python 3.6 support
pip-tools>=6.4.0,<=6.6.2 # Allow earlier versions to retain python 3.6 support
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pin the max version? Or more specifically, why these particular versions for the max allowed?

Ideally this would be a code comment so that someone who comes along later and wonders if it's safe to bump the max allowed version will have more context w/o having to spelunk through git blame.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was mostly to break docker build cache so it would get the latest version of pip-tools, but also this will cause dependabot to manage these dependencies again. Either way, I'm happy to revert this change as it was just to force a rebuild of the helpers.

Copy link
Copy Markdown
Member

@jeffwidman jeffwidman May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also this will cause dependabot to manage these dependencies again.

Pinning makes sense to me for this reason alone, although I'd probably pin a single concrete version rather than a range just for more determinism.

Although on second thought, we still get the determinism if we don't pin at all, we just get whatever pip was shipped with that version of python.

That said, I'm fine either way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the ranges are here again to support earlier python versions. The latest pip and pip-tools have removed support for python 3.6, so if we just run latest, we can't support 3.6. We could pin to the earlier version, but then we have to make sure not to merge any dependabot PRs which will try to pull the pin forward...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, that makes complete sense to keep the range.

@mattt
Copy link
Copy Markdown
Contributor

mattt commented May 27, 2022

@pavera Thanks so much for taking a look at this. What do you think about @jeffwidman's comment? Would removing Python <= 3.6 from PythonVersions::SUPPORTED_VERSIONS and updating pip to =>22.1.1 be a reasonable resolution?

@pavera
Copy link
Copy Markdown
Contributor Author

pavera commented May 27, 2022

As to python 3.6 support, the reason I picked that version for keeping support was that Ubuntu 18.04 which is still supported for another year supports/runs python 3.6. #4958
Totally happy to remove support if that is what we decide is correct though.

As to the workaround itself, I've definitely seen weird issues other than this from running "old" versions of pip (mostly there were a few versions in the 21.X series that had bad bugs in the version resolver that would cause all versions of packages to be downloaded to attempt to resolve a dependency tree). I also don't like the unbounded upgrade this workaround causes, especially since pip has introduced breaking changes on patch releases, so there's risk in both directions.

I could add a version constraint in the workaround, but that feels a bit icky in code.

I have confirmed that Python 3.7 has pip 19, so yes, the cleanest solution is to reject this PR and open a new one that removes python 3.6 and earlier support. Only caveat being we will be removing python support for any users on Ubuntu 18.04 who are using system python.

return if run_command("pyenv versions").include?("#{python_version}\n")

run_command("pyenv install -s #{python_version}")
run_command("pyenv exec pip install --upgrade pip")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I understand how this works... you're first manually saying grab the latest version of pip here, and then in the subsequent command is where it will revert back to a specific pip version?

The original reason for this PR was a broken pip version, so with this current code we could still end up installing a broken pip in the first step, before trying to revert it... but you decided not to pin here because then suddenly we're hardcoding versions within the code rather than the lockfile?
And if a new version of pip ships that's broken, we can temp pin here and then revert once the latest version is unbroken?

I'm totally fine with the route you went, just want to confirm my understanding is correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is all correct. With the additional detail that when pip ships a broken version (they do this at least a couple times a year in my experience), it probably won't be broken for a basic install like the second install here, so we should always be able to revert to the pinned version in the lock file.

@jeffwidman
Copy link
Copy Markdown
Member

jeffwidman commented May 27, 2022

As to python 3.6 support, the reason I picked that version for keeping support was that Ubuntu 18.04 which is still supported for another year supports/runs python 3.6. #4958

Oh yeah, I'd forgotten about that. Also, based on this graph, apparently the world hasn't yet gotten the memo about EOL of 3.6: https://w3techs.com/technologies/history_details/pl-python/3
I don't know how accurate their stats are, but claiming that > 40% of python installs are 3.6 is a big number. So no objections from me to keeping 3.6 around until 18.04 EOL's unless we find other places its costing us tech debt/dev time.

As we discussed, this is a reminder that we should move towards publishing a deprecation schedule ahead of time.

I've also hit pip problems in the past myself for semi-random releases, so pinning the installed version of pip/ pip-tools is something we'll likely want for reasons beyond this particular issue.

Copy link
Copy Markdown
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 from me, you might want to also sync with @mattt before merging in case he has additional thoughts.

@pavera pavera merged commit 1fb6b9f into main May 31, 2022
@pavera pavera deleted the pavera/update-pip-in-pyenv branch May 31, 2022 17:28
@mctofu mctofu mentioned this pull request May 31, 2022
jeffwidman added a commit to jeffwidman/dependabot-core that referenced this pull request Aug 8, 2022
This python helper is being run during the build of `Dependabot-core`
docker image, triggering the following warning:
```
You are using pip version 22.0.4; however, version 22.2.2 is available.
You should consider upgrading via the '/usr/local/.pyenv/versions/3.10.5/bin/python3.10 -m pip install --upgrade pip' command.
```

Beyond the annoyance of the warning message, this means that `pip` is
making an unnecessary call to a remote server to check the version.
A blogger [benchmarked this as costing ~0.2s](https://pythonspeed.com/articles/faster-pip-installs/), so disable it.

This option can also be set globally using a config file or env var:
* https://stackoverflow.com/a/46288945
* https://stackoverflow.com/a/60270281

However, we have a number of different use cases for calling `pip`, and
in some cases we _may_ end up wanting the version check... for example
it feels wrong/unpredictable to simultaneously disable the version check
while we are [upgrading to the latest version](dependabot#5195).
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

Successfully merging this pull request may close these issues.

4 participants