-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Check current python executable version #4520
Check current python executable version #4520
Conversation
I was about to open a new issue for this. Do you know if you'll have a chance to fix the tests on this PR? :-) |
Hey @yokomotod, could you please rebase your PR to resolve the conflict? Thanks a lot for your contribution 👍 fin swimmer |
7682929
to
adc88ef
Compare
adc88ef
to
8b60bf2
Compare
@finswimmer @edmorley done! |
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.
Just a minor nitpick. Otherwise looks good.
src/poetry/utils/env.py
Outdated
) | ||
if not self._poetry.package.python_constraint.allows(current_python): | ||
raise InvalidCurrentPythonVersionError( | ||
self._poetry.package.python_versions, current_python |
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.
current_python
is of type Version
, but the type hint in InvalidCurrentPythonVersionError
says it should be a string. Thus, either the type hint has to be changed or current_python
should be cast to string explicity str(current_python)
. I tend toward the latter.
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.
that's true.
self._poetry.package.python_versions, current_python | |
self._poetry.package.python_versions, str(current_python) |
@radoering Thank you for the review. Can we merge then? |
The tests have to be triggered and succeed first. Unfortunately, I have neither the permissions to trigger the tests nor to merge. |
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.
Thanks a lot 👍 🏅
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Hi.
As #3426 said, current poetry only checks the python constraint in pyproject.toml when creating a virtualenv.
This PR added a python version check for created virtualenv as well, at each command execution.
Demo
(Supporse python 3.8 and 3.9 are installed)
First,
poetry install
withpyproject.tom
which requires python3.8:It creates virtualenv with python 3.8.x executable.
Then change
pyproject.tom
topython = "^3.9"
.With this PR, now
poetry install
aborts with error message:As above message suggests, by running
poetry env use python3.9
,poetry install
will work again.I expect this PR covers all commands which should check python version, like
uninstall
,run
,shell
etc.Pull Request Check List
Resolves: #3426
[ ] Updated documentation for changed code.