gh-127727: Warn when running a virtual environment created for a different minor Python version#149715
gh-127727: Warn when running a virtual environment created for a different minor Python version#149715sepehr-rs wants to merge 8 commits into
Conversation
…imeWarning on minor-version mismatch.
|
All commit authors signed the Contributor License Agreement. |
|
I’m seeing the “Lint / lint” check fail in CI, but I can’t find a concrete error message in the job logs, and |
|
There are lines with whitespaces only but the GH UI may not show them properly. |
|
Thanks a lot, lint check is passing now! |
FFY00
left a comment
FFY00
left a comment
There was a problem hiding this comment.
Thanks @sepehr-rs!
The PR mostly looks okay. I have some questions inline, and the following suggestions:
- Also check
version_info, in addition toversion - Raise a warning on a malformed
versionorversion_infofields - Add test(s)
| if version: | ||
| try: | ||
| major, minor = map(int, version.split(".")[:2]) | ||
| except ValueError: | ||
| major, minor = None | ||
| if ( | ||
| major == sys.version_info.major | ||
| and minor is not None | ||
| and minor != sys.version_info.minor | ||
| and not hasattr(sys, "_venv_version_warning_emitted") | ||
| ): | ||
| _warn( | ||
| f"This virtual environment was created for Python {major}.{minor}, " | ||
| f"but the current interpreter is Python " | ||
| f"{sys.version_info.major}.{sys.version_info.minor}. " | ||
| "Consider running `python -m venv --upgrade` to update the environment.", | ||
| RuntimeWarning, | ||
| ) |
There was a problem hiding this comment.
- Why does the warning only run when the
majorversion is the same? - Where does
sys._venv_version_warning_emittedcome from? Did you forget to include code?
There was a problem hiding this comment.
Why does the warning only run when the
majorversion is the same?
I focused on minor version mismatches since that’s what the issue described. Major version differences (2.x vs 3.x) typically fail more obviously, so I thought a warning would be redundant there. Happy to add it if you think it’s useful though!
Where does sys._venv_version_warning_emitted come from? Did you forget to include code?
Yes, sorry about that! That was leftover from local testing and shouldn’t have been included. I’ve removed it.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Hi @FFY00! Thank you for your kind review and suggestions. |
|
Thanks for making the requested changes! @FFY00: please review the changes made to this pull request. |
3 participants
Attempting to fix #127727, feedback is very welcome!