Implement ExternalPythonOperator

[potiuk]

This Operator works very similarly to PythonVirtualenvOperator - but
instead of creating a virtualenv dynamically, it expects the
Python binary to be available in the environment that Airlfow is run in.

The PR adds not only the implemenat the operator, but also
documents the operator's use and adds best-practices chapter
that explains the differences between different ways how you can
achieve separation of dependencies between different tasks. This
has been a question added many times in by our users, so adding
this operator and outlining future aspects of AIP-46 and AIP-43
that will make separate docker images another option is also
part of this change.

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[potiuk]

I still have to add /fix tests. But for some strange reason I had to make a number of changes to our typing (MyPy complained) - those changes look rather reasonable but @uranusjr maybe you can take a look If I have not made some stupid mistake that led to it.

BTW. PythonExternalOperator seems like a good name overall

[uranusjr]

The main implementation looks fine to me in general but there are too much peripheral changes (mapping changed to mutablemapping etc.) that don’t need to happen.

[potiuk]

The main implementation looks fine to me in general but there are too much peripheral changes (mapping changed to mutablemapping etc.) that don’t need to happen.

Yeah. for some reason when I split - MyPy started to complain on those Mapping to be non Mutable - I just scraped it quickly and simply fixed MyPy, but yeah - I agree I need to find out why MyPy started to complain in the first place.

[potiuk]

I pushed fixes (I still need to add more tests).

Unfortunately it seems that the mypy instabilitites are a bit different nature. As explained in the comment above for some reason it started to complain with those errors:

airflow/operators/python.py:277: error: Argument 3 to "skip" of "SkipMixin" has
incompatible type "Collection[Union[BaseOperator, MappedOperator]]"; expected
"Sequence[BaseOperator]"  [arg-type]
                    self.skip(dag_run, execution_date, downstream_tasks)
                                                       ^
airflow/operators/python.py:282: error: Argument 3 to "skip" of "SkipMixin" has
incompatible type "Iterable[DAGNode]"; expected "Sequence[BaseOperator]"
[arg-type]
    ...              self.skip(dag_run, execution_date, context["task"].get_d...
                                                        ^
Found 2 errors in 1 file (checked 1 source file)

I looked at it closely and I think the suggestions from MyPy were actually correct. I could not find any reason why get_direct_relatives should return DAGNode, as far as I can tell you cannot get TaskGroups - you only get tasks so `Union[BaseOperator, MappedOperator]' (and you cannot skip TaskGroup either). Also Collection was not right, because tasks[0] was used in the 'skip' method:

 DagRun.dag_id == tasks[0].dag_id,

So looks like somethign "masked" the problems from MyPy before and we should fix it here. Any insights and confirmation of my findings would be appreciated before I add more tests.

[raphaelauv]

I think ExternalPythonOperator is a confusing name , that could give the impression to users that the task is going to run out of airflow itself ( it sounds stupid and magical , but so many airflow users are not aware of how things really works and where code is actually running depending the operator and executor )

Why not add a parameter to PythonVirtualenvOperator giving the possibility to the user to set the path of an existing venv ?

or name this new operator PythonExistingVirtualenvOperator ?

thanks

[o-nikolas]

Nice work! Left a few minor comments.

Though I agree with @raphaelauv I don't love the name ExternalPythonOperator and like his proposal as an option. Or maybe PythonPreexistingVirtualenvOperator

[potiuk]

I like the PythonPreexistingVirtualenvOperator name best

[potiuk]

I think this one is ready for review:

• renamed the operator to to PythonPreexistingVirtualenvOperator
• tests should be passing now
• system Test is working (and uses external venv)
• documentation is updated (howtos)
• I even added Best Practices chapter about Handling Python Dependencies that provides more insights on how you can run your tasks using different sets of dependencies - it discusses pros/cons of different approaches

[potiuk]

Ready for final review I think !

[raphaelauv]

would it make sense to add a disclaimer/warning in the case this operator is run in KubernetesExecutor or kubernetesCeleryExecutor (k8S queue) ?

[potiuk]

would it make sense to add a disclaimer/warning in the case this operator is run in KubernetesExecutor or kubernetesCeleryExecutor (k8S queue) ?

What disclaimer? It should work, there are no limits for that. It's perfectly fine to run the operator with KubernetesExecutor IMHO. I can easily imagine this being used when you have just one single image with multiple predefined envs and you want to choose which one to use.

What problems do you foresee with that @raphaelauv ?

[raphaelauv]

Yeah it will work , I'm just concerned about "encouraging" users to create just one single image with multiple predefined envs . Sometimes users create chaotic stack design because something work out of box :)

[potiuk]

Yeah it will work , I'm just concerned about "encouraging" users to create just one single image with multiple predefined envs . Sometimes users create chaotic stack design because something work out of box :)

I am not sure if we want to do it to be honest. I do not think we should encourage it at all (we should present it as an option and we do) because everyone's mileage is different. I spoke to a few users of Airlfow (my customers) and it really depends what stage and experience you have IMHO.

1. some customers who do not have many "system" dependencies problems and are not "super" excited about infrastructure and build pipelinesf for it, will likely prefer single image with multiple venvs (actually the idea of this operator came precisely from that discussion - I met them yesterday and they cannot wait for 2.4 being available because it solves huge problem they have with multiple teams in a simple way).

2. but there are more sophisticated customers who have multiple separate teams that have multiple complex requirements (including system dependencies). There only multiple container images will cut it. Extreme case of it - think GPU and ARM support for one team crossed with having to install Python 2.7 on an old debian because this is the only environment all dependencies will be installable (as much as we would not like it - Python 2.7 is still poptular in gaming industry apparently - Unity added Python binding few years ago with Python 2.7 only https://docs.unity3d.com/Packages/com.unity.scripting.python@2.0/manual/installation.html and it is still 2.7 only !!! 😱 ). This is of course extreme, but you get the idea.

So rather than advising the user to choose one over the other, I chose a different route, similar to the "installation" page - - https://airflow.apache.org/docs/apache-airflow/stable/installation/index.html - if you look at the "best practices" chapter in my PR I simply describe all the options and explain pros and cons of each approach and consequences of choosing each one. This description is precisely targeted for the users who will attempt to ask us "which is the best approach".

Since we cannot answer this question authoritatively IMHO and we do not want to engage in long discussions with each user (this does not scale) to figure out which option is best for the particular user, we will simply send the user to that page, which they will be able to read and decide on their own. We simply cannot make the decisions for them, but we give them all the information in the way that they can make the decision themselves.

I tried to make this "best practices" chapter to be unbiased, factual and very precisely describing pros and cons of each approach and they are grouped in one chapter progresslvely going from the simplest (PythonVirtualenv) to the most complex and involved (Kubernetes). And I tried to avoid any "jugment" there - except the "objective" judgment (more resources used + why) / less resources used +why).

I think this is the best we can do.

[raphaelauv]

thanks for your answer , it's really clear 👍

[potiuk]

thanks for your answer , it's really clear +1

BTW. Funny thing - the customer uses Nomad not K8S, and CeleryExecutor, but it would not change a thing for them if they did use K8S.

[potiuk]

Looking forward to get it merged :)

[potiuk]

:D ?

[potiuk]

Looking forward to merging that one, if there are no more comments. @ashb, you review is blocking here, and I believe the name change is addressed already.

[ashb]

Few small things, pre-emptive approval though.

[potiuk]

Added all changes.