PythonVirtualOperator fails silently when virtualenv is not installed.
Apache Airflow version
2.9.2
If "Other Airflow 2 version" selected, which one?
No response
What happened?
When using PythonVirtualOperator the venv is created by airflow as passed in the dag, in a tmp directory a standard python -m venv /tmp/.... is run. Later pip install -r ... is also run.
However, when virtualenv is not installed the failed command is not properly detected.
Later an uncaught OSError in subprocess.Popen (https://github.com/apache/airflow/blob/c310159bc2363c12110b11febd5febaab8670210/airflow/utils/process_utils.py#L184)is raised by the absent.venv/bin/pipcausing the exit logic not to fire and theTempDirectory.exit` then deletes the evidence.
See: subprocess Exceptions
Additionally stderr is not logged.
What you think should happen instead?
I see in the project file there is a core-all listing including virtualenv.
Shoudn't virtualenv be a dependecy of the pip install apache-airflow? I haven't used hatch
If it shouldn't I think there should be better error handing to explain the issue.
How to reproduce
Run a PythonVirtualOperator with airflow installed in a venv that has no virtualenv.
Operating System
ws2 ubuntu 2404
Versions of Apache Airflow Providers
No response
Deployment
Virtualenv installation
Deployment details
No response
Anything else?
No response
Are you willing to submit PR?
- [ ] Yes I am willing to submit a PR!
Code of Conduct
- [X] I agree to follow this project's Code of Conduct
Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.
Marked it as good first issue for someone to handle.
But we do check
https://github.com/apache/airflow/blob/5f2da71bf86f1f73acd802b66b979e77f53c8715/airflow/operators/python.py#L699-L700
I think @thisiswhereitype to check why it did not work in his case.
Thanks, the reason is that the is_venv_installed finds /usr/bin/virtualenv and proceeds but doesn't try to import it.
A module import would closely match what happens later.
(.venv_airflow) $ python -c 'import importlib.util; importlib.util.find_spec("virtualenv")'
(.venv_airflow) $ which virtualenv
/usr/bin/virtualenv
(.venv_airflow) $ pip list | grep env
<no output>
(.venv_airflow) $
Feel free to fix it, otherwise it will have to wait for someone else to pick it up.
Hey @potiuk, could I take on this issue.
If I am correct, I should just remove the previous checks defined in the function is_venv_installed and add a new check for seeing if it venv is installed or not.
Or should I preserve the previous check and just add a new check with or
What do YOU think is better?
Removing the previous checks defined in the function is_venv_installed and add a new check for seeing if it venv is installed or not as the previous checks are not functioning as desired
Removing the previous checks defined in the function
is_venv_installedand add a new check for seeing if it venv is installed or not as the previous checks are not functioning as desired
Not sure if I got it - but do what you think is best and let's discuss it.
Hi @eladkal , Please assign this to me if @STAR-173 is not working on it.
Hi @pateash , I really appreciate your concern regarding this issue but I think I will just continue to work on this. It's just been busy weeks for a bit. I will be opening a PR within this week
Looks like the PR went stale
Looks like the PR went stale
Wopuld you like to take it over @thisiswhereitype ? Is this what the comment is about? Feel free to do it if you think it's a good idea. We have no problems with even similar people working in parallel on similar PRs - pretty much all the people contributing here are volunteers and they have day jobs, famiilies and hobbies, so they might or might not complete what they initially thought they will be able to do. Usually then someone who is eager to work on it, can take it over, and we can assign it to that person.
Shall I assign it to you @thisiswhereitype ?
@potiuk please do, see the PR
I commented there. You need way more to implement it.