You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/01 12:24:40 UTC
[GitHub] [airflow] nanohanno opened a new issue, #24783: Check if virtualenv is installed fails
nanohanno opened a new issue, #24783:
URL: https://github.com/apache/airflow/issues/24783
### Apache Airflow version
2.3.0
### What happened
When using a `PythonVirtualenvOperator` it is checked if `virtualenv` is installed by
`if not shutil.which("virtualenv"):`
https://github.com/apache/airflow/blob/a1679be85aa49c0d6a7ba2c31acb519a5bcdf594/airflow/operators/python.py#L398
Actually, this expression checks if `virtualenv` is on PATH. If Airflow is installed in a virtual environment itself and `virtualenv` is not installed in the environment the check might pass but `virtualenv` cannot be used as it is not present in the environment.
### What you think should happen instead
It should be checked if `virtualenv` is actually available in the environment.
```python
if importlib.util.find_spec("virtualenv") is None:
raise AirflowException('PythonVirtualenvOperator requires virtualenv, please install it.')
```
https://stackoverflow.com/a/14050282
### How to reproduce
_No response_
### Operating System
Ubuntu 20.04
### Versions of Apache Airflow Providers
_No response_
### Deployment
Virtualenv installation
### Deployment details
_No response_
### Anything else
_No response_
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #24783: Check if virtualenv is installed fails
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #24783: Check if virtualenv is installed fails
URL: https://github.com/apache/airflow/issues/24783
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #24783: Check if virtualenv is installed fails
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #24783:
URL: https://github.com/apache/airflow/issues/24783#issuecomment-1172578717
Yeah good point but I think the root cause is different here. The inconsistency here might come not becaue virtualenv is not reachable via importlib, but because the entrypoint script of the it is not on the PATH (i.e. `bin` of the virtualenv you are in is not added to your PATH).
This indeed might happen, but I think the importlib is not the best way (and most importantly it's not correct way).
If you look at how we are running virtualenv we are doing it via `python -m virtualenv` command - so this is externally launched python interpreted launching virtualenv module. So the current "which" is indeed not correct but running `python -m virtualenv --version` command is actually the best way of checking if virtualenv is installed in the way that will not fail the actual operator's execution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #24783: Check if virtualenv is installed fails
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #24783:
URL: https://github.com/apache/airflow/issues/24783#issuecomment-1172578938
Assigned you @nanohanno
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #24783: Check if virtualenv is installed fails
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #24783:
URL: https://github.com/apache/airflow/issues/24783#issuecomment-1173466130
@uranusjr - see my comment.
When we create the virtualenv, we are actually calling it via `python -m virtualenv` so I think we should not check if virtualenv work in the current interpreter - we are actually creating a new one using `python` when we try to use it - which might, or might not be the same interpreter or might not have PYTHONPATH modified etc. etc.. so safer is to use the same command and create a new interpreter.
Whether it's good or bad to create a new interpreter is debateable but I think if we don't change it, runnning ` python -m virtualenv --version` is the best approach to check. (But yeah I agree with you it's not a "must" - this will be just slightly nicer error to print).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #24783: Check if virtualenv is installed fails
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #24783:
URL: https://github.com/apache/airflow/issues/24783#issuecomment-1173451201
I’d prefer `find_spec` since it is a lot cheaper. If we want to be throughrall (make sure the package actually _works_ instead of just _present_), we can also do
```python
try:
import virtualenv
except ImportError:
# Not installed...
```
which is still much cheaper. But it’s not really necessary IMO, a broken environment is the user’s problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on issue #24783: Check if virtualenv is installed fails
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #24783:
URL: https://github.com/apache/airflow/issues/24783#issuecomment-1621149069
This issue has been automatically marked as stale because it has been open for 365 days without any activity. There has been several Airflow releases since last activity on this issue. Kindly asking to recheck the report against latest Airflow version and let us know if the issue is reproducible. The issue will be closed in next 30 days if no further activity occurs from the issue author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org