Deprecate some functions in the experimental API #19931

ephraimbuddy · 2021-12-01T13:52:25Z

This PR seeks to deprecate some functions in the experimental API.
Some of the deprecated functions are only used in the experimental REST API,
others that are valid are being moved out of the experimental package.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

ephraimbuddy · 2021-12-01T17:12:37Z

airflow/api/client/local_client.py

-        the_pool = pool.get_pool(name=name)
+        the_pool = Pool.get_pool(pool_name=name)
+        if not the_pool:
+            raise PoolNotFound(f"Pool {name} not found")


Previously, the get_pool experimental API raises PoolNotFound when the pool does not exist. Since I have moved it to the Pool model, I don't want it to raise hence raising not found here

Looking at where this function is being called, I wonder if we should just get rid of this exception altogether and just return Optional. Not sure about this.

tests/models/test_pool.py

airflow/api/common/delete_dag.py

airflow/api/client/local_client.py

uranusjr · 2021-12-07T09:52:23Z

airflow/api/common/delete_dag.py

+    try:
+        models_ = [mapper.class_ for mapper in models.base.Base.registry.mappers]
+    except AttributeError:
+        models_ = models.base.Base._decl_class_registry.values()


Might be worthwhile to put this somewhere in airflow.utils with a docstring explaining why it’s needed.

Is this still needed? It was probably a work around for not having SQLA relationships defined.

(I guess we still don't have all of them defined)

ashb

Shouldn't we delete airflow/api/common/experimental/delete_dag.py etc as part of this PR too?

ephraimbuddy · 2021-12-07T11:29:44Z

Shouldn't we delete airflow/api/common/experimental/delete_dag.py etc as part of this PR too?

I was of the opinion that it's a public API and want to deprecate it first and remove it in 2.3.0 but if it's not, I can remove it. Let me know if I should go-ahead

ashb · 2021-12-07T11:39:24Z

Shouldn't we delete airflow/api/common/experimental/delete_dag.py etc as part of this PR too?

I was of the opinion that it's a public API and want to deprecate it first and remove it in 2.3.0 but if it's not, I can remove it. Let me know if I should go-ahead

Yes, good point. Use the same approach as I mention here #18724 (comment)

ashb · 2021-12-14T13:02:33Z

airflow/api/common/experimental/get_dag_run_state.py

 from airflow.api.common.experimental import check_and_get_dag, check_and_get_dagrun


+@deprecated(reason="Use DagRun().get_state() instead", version="2.2.3")


This is still used in airflow/www/api/experimental/endpoints.py -- if we are deprecating it we will need to change those references too.

The experimental API is also deprecated: See

airflow/airflow/www/api/experimental/endpoints.py

Lines 56 to 60 in b20e6d3

def add_deprecation_headers(response: Response):

"""

Add `Deprecation HTTP Header Field

<https://tools.ietf.org/id/draft-dalal-deprecation-header-03.html>`__.

"""

.

My thinking is that if someone is using it externally which may be possible, then we should warn

airflow/models/pool.py

github-actions · 2021-12-15T08:50:48Z

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

This PR seeks to deprecate some functions in the experimental API. Some of the deprecated functions are only used in the experimental REST API, others that are valid are being moved out of the experimental package.

Co-authored-by: Ash Berlin-Taylor <[email protected]>

This PR seeks to deprecate some functions in the experimental API. Some of the deprecated functions are only used in the experimental REST API, others that are valid are being moved out of the experimental package. (cherry picked from commit 6239ae9)

boring-cyborg bot added area:API Airflow's REST/HTTP API area:core-operators Operators, Sensors and hooks within Core Airflow area:webserver Webserver related Issues labels Dec 1, 2021

ephraimbuddy marked this pull request as ready for review December 1, 2021 17:09

ephraimbuddy requested review from ashb, bbovenzi, kaxil, mik-laj, ryanahamilton and XD-DENG as code owners December 1, 2021 17:09

ephraimbuddy commented Dec 1, 2021

View reviewed changes

tests/models/test_pool.py Outdated Show resolved Hide resolved

uranusjr reviewed Dec 7, 2021

View reviewed changes

airflow/api/common/delete_dag.py Outdated Show resolved Hide resolved

ephraimbuddy force-pushed the deprecate-fns branch from c1bc053 to ddf9608 Compare December 7, 2021 09:31

uranusjr reviewed Dec 7, 2021

View reviewed changes

airflow/api/common/delete_dag.py Outdated Show resolved Hide resolved

uranusjr reviewed Dec 7, 2021

View reviewed changes

airflow/api/client/local_client.py Outdated Show resolved Hide resolved

uranusjr reviewed Dec 7, 2021

View reviewed changes

ashb reviewed Dec 7, 2021

View reviewed changes

ephraimbuddy force-pushed the deprecate-fns branch 2 times, most recently from 8e10ec5 to e1a38c6 Compare December 7, 2021 20:02

ephraimbuddy requested review from ashb and uranusjr December 8, 2021 18:35

ashb reviewed Dec 14, 2021

View reviewed changes

airflow/models/pool.py Outdated Show resolved Hide resolved

ashb reviewed Dec 14, 2021

View reviewed changes

airflow/models/pool.py Outdated Show resolved Hide resolved

ephraimbuddy force-pushed the deprecate-fns branch from f717aaf to e03d24b Compare December 14, 2021 17:42

ephraimbuddy requested a review from ashb December 14, 2021 17:42

uranusjr approved these changes Dec 15, 2021

View reviewed changes

github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 15, 2021

ephraimbuddy force-pushed the deprecate-fns branch from e03d24b to c379711 Compare December 15, 2021 09:49

uranusjr approved these changes Dec 15, 2021

View reviewed changes

ephraimbuddy and others added 13 commits December 16, 2021 06:33

Deprecate some functions in the experimental API

6ab9972

This PR seeks to deprecate some functions in the experimental API. Some of the deprecated functions are only used in the experimental REST API, others that are valid are being moved out of the experimental package.

fixup! Deprecate some functions in the experimental API

b2cd44a

fixup! fixup! Deprecate some functions in the experimental API

f1b5685

Update tests/models/test_pool.py

00599d9

support sqlalchemy 1.4

b90bd82

fixup! support sqlalchemy 1.4

55798a0

fixup! fixup! support sqlalchemy 1.4

89b0056

fixup! fixup! fixup! support sqlalchemy 1.4

4d8ea49

mark module as deprecated

afed421

add back deprecated

d111ffa

Update airflow/models/pool.py

89b2a17

Co-authored-by: Ash Berlin-Taylor <[email protected]>

Fix exception raising in pool methods

1ffd06c

check name lenght in local client

0cda3ab

ephraimbuddy force-pushed the deprecate-fns branch from 1931cd9 to 0cda3ab Compare December 16, 2021 09:06

ephraimbuddy merged commit 6239ae9 into apache:main Dec 16, 2021

ephraimbuddy deleted the deprecate-fns branch December 16, 2021 11:30

uranusjr mentioned this pull request Jan 7, 2022

Move mark_tasks out of experiemental, and quelch other deprecation warnings #20742

Closed

jedcunningham added this to the Airflow 2.2.4 milestone Jan 27, 2022

jedcunningham added the type:misc/internal Changelog: Misc changes that should appear in change log label Feb 17, 2022

jedcunningham mentioned this pull request Feb 18, 2022

Status of testing of Apache Airflow 2.2.4rc1 #21669

Closed

40 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate some functions in the experimental API #19931

Deprecate some functions in the experimental API #19931

ephraimbuddy commented Dec 1, 2021

ephraimbuddy Dec 1, 2021

uranusjr Dec 7, 2021 •

edited

Loading

uranusjr Dec 7, 2021

ashb Dec 7, 2021

ashb left a comment

ephraimbuddy commented Dec 7, 2021

ashb commented Dec 7, 2021

ashb Dec 14, 2021

ephraimbuddy Dec 14, 2021

github-actions bot commented Dec 15, 2021

		from airflow.api.common.experimental import check_and_get_dag, check_and_get_dagrun


		@deprecated(reason="Use DagRun().get_state() instead", version="2.2.3")

	def add_deprecation_headers(response: Response):
	"""
	Add `Deprecation HTTP Header Field
	<https://tools.ietf.org/id/draft-dalal-deprecation-header-03.html>`__.
	"""

Deprecate some functions in the experimental API #19931

Deprecate some functions in the experimental API #19931

Conversation

ephraimbuddy commented Dec 1, 2021

ephraimbuddy Dec 1, 2021

Choose a reason for hiding this comment

uranusjr Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

uranusjr Dec 7, 2021

Choose a reason for hiding this comment

ashb Dec 7, 2021

Choose a reason for hiding this comment

ashb left a comment

Choose a reason for hiding this comment

ephraimbuddy commented Dec 7, 2021

ashb commented Dec 7, 2021

ashb Dec 14, 2021

Choose a reason for hiding this comment

ephraimbuddy Dec 14, 2021

Choose a reason for hiding this comment

github-actions bot commented Dec 15, 2021

uranusjr Dec 7, 2021 •

edited

Loading