Add Airflow specific warning classes #25799

eladkal · 2022-08-18T15:23:21Z

Adding ~~RemoveInAirflow3DeprecationWarning~~RemovedInAirflow3Warning and AirflowProviderDeprecationWarning

RemovedInAirflow3Warning - modifying all occurrences of DeprecationWarning in core to the new class.
AirflowProviderDeprecationWarning - only adding the class. We can not yet replace warnings in providers. This will be useful when minimum provider version would be on Airflow 2.4 (or the release of this PR)

Regarding contrib folder due to the discussion of [DISCUSS] Move "contrib" and all old classes to a separate package in mailing list I left all classes untouched they will still produce DeprecationWarning

Closes: #22356

Todo:

Verify no missing directories (except contrib)
fix tests of deprecation warnings
We have some occurrences of PendingDeprecationWarning in the code base ~~but I'm not sure if we have that distinction? Should we also have RemovedInAirflow3Warning ?~~
Providers: Replaced all PendingDeprecationWarning with DeprecationWarning
Core: Replaced all PendingDeprecationWarning with RemovedInAirflow3Warning

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

airflow/configuration.py

uranusjr

I didn’t read everything

ashb · 2022-08-19T11:22:24Z

airflow/exceptions.py

@@ -338,3 +338,17 @@ class TaskDeferralError(AirflowException):

 class PodReconciliationError(AirflowException):
    """Raised when an error is encountered while trying to merge pod configs."""
+
+
+class RemoveInAirflow3DeprecationWarning(DeprecationWarning):


https://docs.python.org/3/library/warnings.html

DeprecationWarning Base category for warnings about deprecated features when those warnings are intended for other Python developers (ignored by default, unless triggered by code in main).

FutureWarning Base category for warnings about deprecated features when those warnings are intended for end users of applications that are written in Python.

-- --

PendingDeprecationWarning Base category for warnings about features that will be deprecated in the future (ignored by default).

-- --

Should we make this baseclass be FutureWarning?

We consider Airflow users as Python developers not end users don't we?

PendingDeprecationWarnings and DeprecationWarnings are filtered out by default. ~~I think subclassing from FutureWarning would make them more visible to Airflow DAG authors.~~

However, we may not want this to be immediately visible to DAG authors, because at this point we should still assume that we will only have 2.x releases.

Subclassing from DeprecationWarning (or PendingDeprecationWarning) might actually be the right call here until we have concrete plans for Airflow 3.0. PEP-0387:

If an API is being removed, simply warn whenever it is entered. DeprecationWarning is the usual warning category to use, but PendingDeprecationWarning may be used in special cases where the old and new versions of the API will coexist for many releases [2].

(emphasis mine)

We explicitly enable the Deprecation warnings coming from airflow following https://peps.python.org/pep-0565/#recommended-filter-settings-for-interactive-shells

See airflow.configuration.py:

if not sys.warnoptions: warnings.filterwarnings(action='default', category=DeprecationWarning, module='airflow') warnings.filterwarnings(action='default', category=PendingDeprecationWarning, module='airflow')

I'm curious why #1285 "turned on" both DeprecationWarning and PendingDeprecationWarning.

It was 6 years ago :) 5 Apr 2016 :). I am afraid no commiters who are active now were even around :D

sorry to comment here, but i'm obsessed with DeprecationWarning recently because i couldn't find mine 😂 :
https://apache-airflow.slack.com/archives/CSS36QQS1/p1661974104498239

airflow/exceptions.py

potiuk · 2022-08-22T23:52:09Z

Comment: We COULD potentially do the same approach as with common.sql and introduce 'apache-airflow-providers-common-provider` and add the AirflowProviderDeprecationWarning.

If the package is very small, we can keep it as a way to add functionality that is common to all providers.

And we could replace the provider's warnings even now.

potiuk · 2022-08-22T23:57:38Z

Should we add a pre-commit to check automatically if there are no "standard" warnings in Airflow code? I think it should be as easy as using pygrep pre-commit (we already use it in a few places).

eladkal · 2022-08-23T05:25:38Z

And we could replace the provider's warnings even now.

We can but to be honest I think it has very little value. With the new provider governance module there is no reason to keep deprecations for so long. We can remove them really fast even in a follow up release with is 1 month window and cherry pick features to earlier releases.
Also, after 2.4 is minimum Airflow version for providers you probably won't need apache-airflow-providers-common-provider to store the deprecation class as you will just import it from core so I don't think it's worth the effort

eladkal · 2022-08-23T08:25:36Z

Should we add a pre-commit to check automatically if there are no "standard" warnings in Airflow code? I think it should be as easy as using pygrep pre-commit (we already use it in a few places).

Added

Adding `RemoveInAirflow3DeprecationWarning` and `AirflowProviderDeprecationWarning` Closes: apache#22356

boring-cyborg bot added provider:cncf-kubernetes Kubernetes provider related issues area:logging area:Scheduler including HA (high availability) scheduler area:secrets area:serialization labels Aug 18, 2022

eladkal commented Aug 18, 2022

View reviewed changes

airflow/configuration.py Outdated Show resolved Hide resolved

eladkal marked this pull request as ready for review August 18, 2022 18:02

eladkal requested review from dstandish, kaxil, potiuk, ashb, uranusjr, jhtimmins, ryanahamilton, bbovenzi, jedcunningham and XD-DENG as code owners August 18, 2022 18:02

eladkal changed the title ~~[WIP] Add Airflow specific warning classes~~ Add Airflow specific warning classes Aug 18, 2022

eladkal added this to the Airflow 2.4.0 milestone Aug 18, 2022

uranusjr approved these changes Aug 19, 2022

View reviewed changes

ashb reviewed Aug 19, 2022

View reviewed changes

eladkal force-pushed the warn branch from a54ee21 to 15e8826 Compare August 21, 2022 07:13

blag reviewed Aug 22, 2022

View reviewed changes

airflow/exceptions.py Outdated Show resolved Hide resolved

Add Airflow specific warning classes

9c3cc36

Adding `RemoveInAirflow3DeprecationWarning` and `AirflowProviderDeprecationWarning` Closes: apache#22356

eladkal force-pushed the warn branch from 8d175f7 to 9c3cc36 Compare August 23, 2022 09:05

eladkal requested a review from ashb August 23, 2022 10:15

eladkal requested a review from blag August 23, 2022 10:15

potiuk approved these changes Aug 23, 2022

View reviewed changes

potiuk merged commit 1a1f352 into apache:main Aug 23, 2022

blag approved these changes Aug 23, 2022

View reviewed changes

eladkal deleted the warn branch August 23, 2022 22:12

jedcunningham added the type:new-feature Changelog: New Features label Sep 12, 2022

potiuk mentioned this pull request Nov 15, 2022

Status of testing Providers that were prepared on November 15, 2022 #27674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Airflow specific warning classes #25799

Add Airflow specific warning classes #25799

eladkal commented Aug 18, 2022 •

edited

Loading

uranusjr left a comment

ashb Aug 19, 2022 •

edited

Loading

eladkal Aug 21, 2022

blag Aug 23, 2022 •

edited

Loading

blag Aug 23, 2022

potiuk Aug 23, 2022

blag Aug 23, 2022

potiuk Aug 23, 2022

zachliu Aug 31, 2022 •

edited

Loading

potiuk commented Aug 22, 2022 •

edited

Loading

potiuk commented Aug 22, 2022

eladkal commented Aug 23, 2022

eladkal commented Aug 23, 2022

DeprecationWarning	Base category for warnings about deprecated features when those warnings are intended for other Python developers (ignored by default, unless triggered by code in main).
FutureWarning	Base category for warnings about deprecated features when those warnings are intended for end users of applications that are written in Python.
--	--
PendingDeprecationWarning	Base category for warnings about features that will be deprecated in the future (ignored by default).
--	--

Add Airflow specific warning classes #25799

Add Airflow specific warning classes #25799

Conversation

eladkal commented Aug 18, 2022 • edited Loading

uranusjr left a comment

Choose a reason for hiding this comment

ashb Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

eladkal Aug 21, 2022

Choose a reason for hiding this comment

blag Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

blag Aug 23, 2022

Choose a reason for hiding this comment

potiuk Aug 23, 2022

Choose a reason for hiding this comment

blag Aug 23, 2022

Choose a reason for hiding this comment

potiuk Aug 23, 2022

Choose a reason for hiding this comment

zachliu Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

potiuk commented Aug 22, 2022 • edited Loading

potiuk commented Aug 22, 2022

eladkal commented Aug 23, 2022

eladkal commented Aug 23, 2022

eladkal commented Aug 18, 2022 •

edited

Loading

ashb Aug 19, 2022 •

edited

Loading

blag Aug 23, 2022 •

edited

Loading

zachliu Aug 31, 2022 •

edited

Loading

potiuk commented Aug 22, 2022 •

edited

Loading