-
Notifications
You must be signed in to change notification settings - Fork 7k
[ci] pin pip version in min setup #58215
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
Conversation
Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
| "${python}" -m pip install --upgrade pip | ||
| # 25.3 has breaking change where other Python packages like "click" does not work | ||
| # with it anymore. pip-compile will fail to work with the package's setup code. | ||
| "${python}" -m pip install pip==25.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request pins the pip version to 25.2 to fix a build failure caused by a breaking change in a newer version. The change is straightforward and should resolve the issue. My review includes a suggestion to improve the comment for better clarity and long-term maintainability by adding a TODO and generalizing the explanation to avoid potential confusion with the unconventional version numbers used.
| # 25.3 has breaking change where other Python packages like "click" does not work | ||
| # with it anymore. pip-compile will fail to work with the package's setup code. | ||
| "${python}" -m pip install pip==25.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinning pip is a good immediate fix for the build issue. For better long-term maintainability and clarity, I have a couple of suggestions.
First, the version numbers 25.3 and 25.2 are unconventional for pip, which usually follows a YY.N format (e.g., 24.1). This can be confusing for future maintainers.
Second, it's a good practice to add a TODO to track temporary fixes like this, making it easier to unpin the version once the upstream issue is resolved.
Here's a suggested change that incorporates these points by making the comment more general and adding a TODO:
| # 25.3 has breaking change where other Python packages like "click" does not work | |
| # with it anymore. pip-compile will fail to work with the package's setup code. | |
| "${python}" -m pip install pip==25.2 | |
| # TODO(ci): Unpin pip once the upstream breaking change is fixed. See [link to issue if available]. | |
| # A recent pip version introduced a breaking change that affects packages like "click" | |
| # and causes pip-compile to fail. Pinning to a known-good version as a workaround. | |
| "${python}" -m pip install pip==25.2 |
lint on code format and cherrypick #58215 --------- Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
fixes min setup build Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
fixes min setup build Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
fixes min setup build