Add support for PackageIndex connections in PythonVirtualenvOperator
#52288
Conversation
There was a problem hiding this comment.
Oh, very cool! I am wondering that nobody else was missing this until today!
Can I kindly request that (1) you add a pytest to ensure that passed parameters are correctly resolved in the list and (2) that the feature is also visible in the documentation in providers/standard/docs/operators/python.rst describing also "why" and when to use it?
As I assume the benefit of the passing of connection IDs is that no Jinja templating is needed but this would lead to a "late resolution" of connections, what happens if the named connection is not existing? Task will fail and what will be the presented error message? Might there need to be a bit exception handling/error hints being needed? Or how would the error look like if connection is not existing?
providers/standard/src/airflow/providers/standard/operators/python.py
Outdated
Show resolved
Hide resolved
0f9098e to
6643e8b
Compare
Thanks for the review! I just added a unit test and extended the documentation.
The error message is already relatively clear I think. Or should we add a custom exception to extend the error message? |
There was a problem hiding this comment.
Looks great in my view. Thanks! Just catced some conflict needed to be resolved, probably due to Python 3.9 deprecation.
As I was having a very detailled review initially when I was contributing this base, would ask some additional pair of eyes... @potiuk you were the reviewer in the past - is this okay also with an extra eye on security?
…or-package-index-connections-to-PythonVirtualenvOperator
…or-package-index-connections-to-PythonVirtualenvOperator
Description
The
PythonVirtualenvOperatoroffers the option to pass additionalindex_urlsthat are used as additional package sources. Furthermore there is thePackageIndexHookwhich allows users to define Python package sources as Airflow connection. They can put the index URL and also credentials for this index URL in there. See here: https://airflow.apache.org/docs/apache-airflow-providers-standard/stable/_api/airflow/providers/standard/hooks/package_index/index.htmlHowever there is currently no "clean" way to pass a
PackageIndexconnection into thePythonVirtualenvOperator. This PR adds this integration to thePythonVirtualenvOperator.^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.