Skip to content

Fixing PR failures if pypi.org unavailable#5876

Merged
pavera merged 6 commits intomainfrom
pavera/python-firewalled
Oct 17, 2022
Merged

Fixing PR failures if pypi.org unavailable#5876
pavera merged 6 commits intomainfrom
pavera/python-firewalled

Conversation

@pavera
Copy link
Copy Markdown
Contributor

@pavera pavera commented Oct 12, 2022

Context

There are many ways to specify an alternate pypi index in Python depending on the package manager, for the most part we attempt to parse these settings from pyproject.toml, pip.conf, and requirements.txt settings. If the project specifies an alternate index url we shouldn't call the public pypi.org index.

There are a number of calls we make to pypi.org to retrieve data that is only available there (primarily the <package name>/json metadata endpoint). These calls currently cause PR creation to fail if pypi.org is unavailable or otherwise blocked/firewalled. We should be able to generate a PR given a private index even if pypi.org is unavailable.

Approach

There are 3 classes of issues I found in attempting to run updates without pypi.org access:

  1. Failures to get metadata from pypi.org resulted in failed PR creation
  2. Our python version detection was failing to detect already installed python versions resulting in extra attempts to install python and pip from pypi.org
  3. pip-compile is not properly injecting an alternate index url provided in a requirements.txt file

I've added a couple rescues to catch timeouts so failed attempts to reach pypi.org do not crash pr creation. These could probably be expanded further to include other network errors (SocketError maybe?)

Calls to pyenv versions were always failing to find installed python for pipenv and pip-compile as they were expecting a newline which is no longer in the output. This "fix" only resolves the issue if the repo is using a version of python we have pre-installed.

I have not attempted to address point 3 above yet, I added a partial solution of exposing the main_url from IndexFinder via a new public method on IndexFinder and this worked, however it would require a larger change to properly set this in context with replaces_base and I feel there might be better options to explore around parsing the exact command pip-compile recommends from the requirements.txt file. Since pip-compile supports multiple source files (requirements.in, setup.py, and pyproject.toml) we should probably take a more holistic approach here.

@pavera pavera requested a review from a team as a code owner October 12, 2022 14:17
@pavera pavera force-pushed the pavera/python-firewalled branch from e77df7b to 58bd5eb Compare October 13, 2022 13:21
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the preceding space important here? If so, what do you think about using "\ #{? I'm able to notice it here because of diff highlighting, but I'd be much less likely to notice it when I need to.

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 the space is important, I've applied the suggested change.

@pavera pavera force-pushed the pavera/python-firewalled branch 2 times, most recently from a5e1ae6 to d6f9c66 Compare October 17, 2022 19:57
@pavera pavera force-pushed the pavera/python-firewalled branch from d6f9c66 to 8467689 Compare October 17, 2022 20:59
@pavera pavera merged commit 10f46df into main Oct 17, 2022
@pavera pavera deleted the pavera/python-firewalled branch October 17, 2022 22:37
@pavera pavera mentioned this pull request Oct 31, 2022
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.

2 participants