Standard provider python operator
Moving python operators/sensors to standard provider
^ 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.
cc: @romsharon98
Some of the tests are failing due to compatibility issues. It appears that the changes related to the triggering_asset_events event are associated with Airflow 3.0.0. Since we're transitioning the Python operator into the providers' ecosystem, the standard provider now requires a minimum Airflow version of 2.10.0. As a result, the compatibility tests for version 2.10.1 are failing. Any suggestions ?
https://github.com/apache/airflow/actions/runs/11308098417/job/31450685352?pr=42081#step:11:12372
@potiuk It appears that this PR needs to be initiated from the Apache repository, or it should be someone with committer privileges. The PR involves modifications in the dev/breeze and dev/tests_common/test_utils/compat.py sections, but it seems that the CI process is overriding these scripts with those from the main branch, which could be causing some issues.
Here it is removing https://github.com/apache/airflow/actions/runs/11308098408/job/31450591466#step:4:219
https://github.com/apache/airflow/pull/42081/files#diff-cbadf25516149c7fe3d19e45b5b03789f4bb93bd6d837eaa365dfd0ffb771a0fR55
For dev/tests_common/test_utils/compat.py have workaround but dont like this, i have imported operators with try/catch like this https://github.com/apache/airflow/pull/42081/files#diff-a842cdd5bcf15ce8859d6115b48853c3ac654701a3279bce73969be8922dbbe8R47. these imports can be replaced using compat.
Any thoughts please ?
@potiuk It appears that this PR needs to be initiated from the Apache repository, or it should be someone with committer privileges. The PR involves modifications in the
dev/breezeanddev/tests_common/test_utils/compat.pysections, but it seems that the CI process is overriding these scripts with those from the main branch, which could be causing some issues.Here it is removing https://github.com/apache/airflow/actions/runs/11308098408/job/31450591466#step:4:219
https://github.com/apache/airflow/pull/42081/files#diff-cbadf25516149c7fe3d19e45b5b03789f4bb93bd6d837eaa365dfd0ffb771a0fR55
For
dev/tests_common/test_utils/compat.pyhave workaround but dont like this, i have imported operators with try/catch like this https://github.com/apache/airflow/pull/42081/files#diff-a842cdd5bcf15ce8859d6115b48853c3ac654701a3279bce73969be8922dbbe8R47. these imports can be replaced using compat.Any thoughts please ?
looks like the tests are going through fine now.
@potiuk It appears that this PR needs to be initiated from the Apache repository, or it should be someone with committer privileges. The PR involves modifications in the
dev/breezeanddev/tests_common/test_utils/compat.pysections, but it seems that the CI process is overriding these scripts with those from the main branch, which could be causing some issues.Here it is removing https://github.com/apache/airflow/actions/runs/11308098408/job/31450591466#step:4:219
https://github.com/apache/airflow/pull/42081/files#diff-cbadf25516149c7fe3d19e45b5b03789f4bb93bd6d837eaa365dfd0ffb771a0fR55
For
dev/tests_common/test_utils/compat.pyhave workaround but dont like this, i have imported operators with try/catch like this https://github.com/apache/airflow/pull/42081/files#diff-a842cdd5bcf15ce8859d6115b48853c3ac654701a3279bce73969be8922dbbe8R47. these imports can be replaced using compat.Any thoughts please ?
looks like tests are running..now..
Compat tests 2.10.1 failing because python operator has changes related to airflow 3.0.0, should i update back changes to support in 2.10.X?
Compat tests 2.10.1 failing because python operator has changes related to airflow 3.0.0, should i update back changes to support in 2.10.X?
It looks like it's more of "unit test compatibility" than backporting it. I think in this case simply tests should be fixed to be able to handle past providers. We do not need to backport this change, but we need to make sure that the assertions work when airflow is installed from "2.10.1"
Compat tests 2.10.1 failing because python operator has changes related to airflow 3.0.0, should i update back changes to support in 2.10.X?
It looks like it's more of "unit test compatibility" than backporting it. I think in this case simply tests should be fixed to be able to handle past providers. We do not need to backport this change, but we need to make sure that the assertions work when
airflowis installed from "2.10.1"
thanks jarek, When installing Airflow 2.10.1, the Python operator is included as part of the core package. Should assertions be checked against the core Python operator (i.e., airflow.operators.python) or against airflow.providers.standard.operators.python? Opting for the former would require code changes due to the adjustments already made for Airflow 3. Do you agree that if Airflow 2.10.1 is installed, validation tests should be conducted on airflow.operators.python, as long as the tests remain compatible?
Compat tests 2.10.1 failing because python operator has changes related to airflow 3.0.0, should i update back changes to support in 2.10.X?
It looks like it's more of "unit test compatibility" than backporting it. I think in this case simply tests should be fixed to be able to handle past providers. We do not need to backport this change, but we need to make sure that the assertions work when
airflowis installed from "2.10.1"thanks jarek, When installing Airflow 2.10.1, the Python operator is included as part of the core package. Should assertions be checked against the core Python operator (i.e., airflow.operators.python) or against airflow.providers.standard.operators.python? Opting for the former would require code changes due to the adjustments already made for Airflow 3. Do you agree that if Airflow 2.10.1 is installed, validation tests should be conducted on airflow.operators.python, as long as the tests remain compatible?
ah i got it what you mean by 'assertions work', need to make some adjustments in the python operator tests related to new feature updates. hope that works..
ah i got it what you mean by 'assertions work', need to make some adjustments in the python operator tests related to new feature updates. hope that works..
Yes. Some examples and detailed explanation on how to do this "test_compatibility" are here: https://github.com/apache/airflow/blob/main/contributing-docs/testing/unit_tests.rst#implementing-compatibility-for-provider-tests-for-older-airflow-versions
I believe we have to move these scripts as well into standard provider airflow/utils/python_virtualenv.py airflow/utils/python_virtualenv_script.jinja2 ?
I believe we have to move these scripts as well into standard provider
airflow/utils/python_virtualenv.pyairflow/utils/python_virtualenv_script.jinja2?
Yes. Absolutely.
I believe we have to move these scripts as well into standard provider
airflow/utils/python_virtualenv.pyairflow/utils/python_virtualenv_script.jinja2?Yes. Absolutely.
cool, Thank you :)
Just one thing about compat provider reuse.
yeah am updating imports to use compat.
Just one thing about compat provider reuse.
yeah am updating imports to use compat.
I've added the imports to the compat provider, but I've noticed test failures occurring in selective checks. This is likely because all provider imports are now referenced directly from the compat provider. So, when we revert to standard provider direct imports, we may need to update all the selective check tests accordingly.
Is it worth adding to compat provider now?
Just one thing about compat provider reuse.
yeah am updating imports to use compat.
I've added the imports to the compat provider, but I've noticed test failures occurring in selective checks. This is likely because all provider imports are now referenced directly from the compat provider. So, when we revert to standard provider direct imports, we may need to update all the selective check tests accordingly.
No - it's ok - you need to fix the tests accordingly. Those tests are testing whether the right set of "dependent" providers are selected when selective checks is running - so when you start depending on the standard provider and when standard provider depends on some other provider., the tests start to expect changes in the depednencies, so what needs to be done is to update the tests to reflect that.
And it's not that difficult, most of the code is common in constants, and some other could be replaced with search/replace.
Just one thing about compat provider reuse.
yeah am updating imports to use compat.
I've added the imports to the compat provider, but I've noticed test failures occurring in selective checks. This is likely because all provider imports are now referenced directly from the compat provider. So, when we revert to standard provider direct imports, we may need to update all the selective check tests accordingly.
No - it's ok - you need to fix the tests accordingly. Those tests are testing whether the right set of "dependent" providers are selected when selective checks is running - so when you start depending on the standard provider and when standard provider depends on some other provider., the tests start to expect changes in the depednencies, so what needs to be done is to update the tests to reflect that.
Sure , thanks jarek. Will push the changes..
BTW. ... some conflicts to solve - so you need to rebase too
BTW. ... some conflicts to solve - so you need to rebase too
Thanks jarek, finally this is green now.
One failure related to spell check this will fix https://github.com/apache/airflow/pull/43538
Are we good to merge this ? 😄
Go ahead and do the honors @gopidesupavan -> That's an interesting one as first(?) serious one to merge as a committer.. Don't be shy. We can always revert it if it breaks things ;)
Go ahead and do the honors @gopidesupavan -> That's an interesting one as first(?) serious one to merge as a committer.. Don't be shy. We can always revert it if it breaks things ;)
woohoooo merged thank you all for the help and review here, really appreciate it 😄