Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate passing execution_date to XCom methods #19825

Merged
merged 12 commits into from
Dec 15, 2021

Conversation

ashb
Copy link
Member

@ashb ashb commented Nov 25, 2021

As part of AIP-39 (released in 2.2) we added run_id parameters to XCom
methods, and this changes it so that passing by run_id is the
recommended approach.

A future PR will change the columns on the xcom table to store run_id
(instead/as well as exeuction_date) but that will be for 2.3, where as
this change can be backported to 2.2.x

Discussion thread: https://lists.apache.org/thread/gofj3g6m6vvksy6n0cmgq1qxd309bbbl

tests/models/test_xcom.py Outdated Show resolved Hide resolved
airflow/models/taskinstance.py Outdated Show resolved Hide resolved
airflow/models/xcom.py Outdated Show resolved Hide resolved
ashb and others added 3 commits December 2, 2021 18:11
As part of AIP-39 (released in 2.2) we added `run_id` parameters to XCom
methods, and this changes it so that passing by run_id is the
recommended approach.

A future PR will change the columns on the xcom table to store run_id
(instead/as well as exeuction_date) but that will be for 2.3, where as
this change can be backported to 2.2.x
@uranusjr uranusjr force-pushed the depcreate-xcom-execution_date branch from 5509eeb to 2c8862f Compare December 2, 2021 11:40
tests/models/test_xcom.py Outdated Show resolved Hide resolved
@uranusjr uranusjr force-pushed the depcreate-xcom-execution_date branch from 486b76e to 2d2b72d Compare December 2, 2021 18:26
Both the "modern" run_id and deprecated execution_date approaches.
@uranusjr uranusjr force-pushed the depcreate-xcom-execution_date branch from 2bb7bcd to fdef902 Compare December 3, 2021 05:23
@uranusjr
Copy link
Member

uranusjr commented Dec 3, 2021

I’ve made all tests work and migrated most existing XCom usages to use run_id. One (very big) exception is operator link; since get_link is publicly subclassable, we can’t just change the argument. The current plan (after discussing with @ashb) is to add an optional flag on the class to toggle the behaviour, but there are much detail (and likely bikeshedding) around that, so I’m going to do it in a separate PR. The depracation warning implemented in this PR on passing execution_date to XCom is changed from DeprecationWarning to PendingDeprecationWarning to reflect that—we won’t commit the deprecation until we provide a migration path for operator links (which should still happen in 2.3, but let’s not jump the gun).

I also want to extract typing changes I made here into a separate PR, and merge that first before I mark this PR as ready for review.

@uranusjr
Copy link
Member

uranusjr commented Dec 7, 2021

Merging in the Mypy and Sphinx fixes, let’s see this works.

@uranusjr
Copy link
Member

uranusjr commented Dec 7, 2021

Oh everything is 🟢 now!

@uranusjr uranusjr marked this pull request as ready for review December 7, 2021 15:01
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

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.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Dec 7, 2021
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

Should we add a note in UPDATING.md too plz around the deprecation

@uranusjr
Copy link
Member

uranusjr commented Dec 9, 2021

UPGRADING.md entry added. I think we also want to migrate XCom to add the run_id column in 2.3, and the upgrading note will probably be changed when that is implemented.

@ashb ashb merged commit 5712e2b into apache:main Dec 15, 2021
@ashb ashb deleted the depcreate-xcom-execution_date branch December 15, 2021 13:08
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants