-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
Fail if Specified Interpreter Not Found #2047
Fail if Specified Interpreter Not Found #2047
Conversation
42f1aba
to
99c2e54
Compare
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.
How is this taking into account https://tox.readthedocs.io/en/latest/config.html#conf-skip_missing_interpreters?
|
||
if path is None: | ||
print('envname {} not found'.format(envconfig.envname)) | ||
sys.exit(1) |
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.
You're not supposed to use sys.exit here, instead raise an exception.
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.
Can you say which exception could be raised to give a concise error message back to the user?
I have tried raising tox.exception.UnsupportedInterpreter but that presented a lot of error text to the user.
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.
Would need to investigate, but that does seem the right exception type. Also, I don't think this should be handled here, but higher up in the call chain.
@jackdesert I think this is a misunderstanding $ tox
py31 create: /tmp/y/.tox/py31
ERROR: InterpreterNotFound: python3.1
___________________________________ summary ____________________________________
ERROR: py31: InterpreterNotFound: python3.1 |
(Do NOT use basepython as a fallback) This is intended to remedy these two github issues: tox-dev#882 tox-dev#1484 by :user: jackdesert
d20484c
to
30642f6
Compare
for more information, see https://pre-commit.ci
Would it be straightforward to create a pull request that raises an error if an improper env name is used? |
no, envnames can be literally anything -- only a handful have special meaning ( |
How can this be paved to be clear to future users? It was frustrating to be attempting to specify python 3.1 and having it choose the system python instead. |
I don't know how this could be more clear! no part of our documentation nor the quick start nor any of the thousands of examples in open source use names like what you've used -- I guess the better question is how did you arrive to your naming? |
Not sure. I am using pyenv, which uses periods. |
Fail if specified interpreter not found.
(Do NOT use basepython as a fallback)
This is intended to remedy these two github issues:
#882
#1484
In particular, when python3.1 is NOT installed on my machine, using this tox.ini:
this pull request makes is so that tox fails.
(Without this commit, tox runs pytest using the same python interpreter that was used to invoke tox.
In my opinion it is more helpful to fail than to run tests with a different-than-intended interpreter.)
If this pull request seems generally useful, let me know what it would take to get it accepted. (It broke several tests, for example.)