Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Dec 7, 2023

Templating of Python-based decorators has been broken since implementation. The decorators used template_fields definition as defined originally in PythonOperator rather than the ones from virtualenv because template fields were redefined in _PythonDecoratedOperator class and they took precedence (MRU).

This PR add explicit copying of template_fields from the operators that they are decorating.

Fixes: #36102


^ 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.

Templating of Python-based decorators has been broken since
implementation. The decorators used template_fields definition
as defined originally in PythonOperator rather than the ones from
virtualenv because template fields were redefined in
_PythonDecoratedOperator class and they took precedence (MRU).

This PR add explicit copying of template_fields from the operators
that they are decorating.

Fixes: apache#36102
@potiuk potiuk requested a review from uranusjr as a code owner December 7, 2023 07:44
@potiuk potiuk added this to the Airflow 2.8.0 milestone Dec 7, 2023
@uranusjr
Copy link
Member

uranusjr commented Dec 7, 2023

I feel this entire hierarchy of operators needs some revamp, the inheritance is getting too deep and there are too many operator classes. But this is a good first step since it at least gets the behaviour right.

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking a similar approach to how fsspec reduces the need of n2 operators to 1 operator + n backends.

@potiuk
Copy link
Member Author

potiuk commented Dec 7, 2023

Yeah. It's a bit clunky :)

@potiuk potiuk merged commit 3904206 into apache:main Dec 7, 2023
@potiuk potiuk deleted the fix-temmplating-requirement-python-virtualenv branch December 7, 2023 09:19
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 10, 2024
@ephraimbuddy ephraimbuddy added type:bug-fix Changelog: Bug Fixes and removed changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) labels Jan 10, 2024
ephraimbuddy pushed a commit that referenced this pull request Jan 11, 2024
Templating of Python-based decorators has been broken since
implementation. The decorators used template_fields definition
as defined originally in PythonOperator rather than the ones from
virtualenv because template fields were redefined in
_PythonDecoratedOperator class and they took precedence (MRU).

This PR add explicit copying of template_fields from the operators
that they are decorating.

Fixes: #36102
(cherry picked from commit 3904206)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug-fix Changelog: Bug Fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using requirements file in VirtualEnvPythonOperation appears to be broken

3 participants