Protect against accidental misuse of XCom.get_value() #22244
Conversation
potiuk
requested review from
ashb,
mik-laj,
jedcunningham and
turbaszek
as code owners
boring-cyborg
bot
added
area:dev-tools
area:providers
kind:documentation
provider:amazon-aws
potiuk
force-pushed
the
add-protection-against-incompatible-xcom-getvalue-usage
branch
from
138827d to
b9b1b11
Compare
|
Updated it so the error looks a bit nicer:
|
potiuk
force-pushed
the
add-protection-against-incompatible-xcom-getvalue-usage
branch
from
b9b1b11 to
6161beb
Compare
|
I'd love to merge it quickly so that I could incorporate this in the changelogs (hidden entry but avoids future release pollution) to include that one. |
| @@ -244,7 +244,7 @@ def get_link( | |||
| :param dttm: datetime | |||
| :return: url link | |||
| """ | |||
| if ti_key: | |||
| if ti_key is not None: | |||
There was a problem hiding this comment.
This makes no difference does it? (since we don't define a custom book on TIKey)
There was a problem hiding this comment.
Nope. But it's easier to standardize as convention. Both ways were used in the code and I arbitrary chose not None as more explicit.
There was a problem hiding this comment.
This way we have only one "standard" way to check.
|
Random doc inventory problem it seems. Reopened to rebuild. |
github-actions
bot
added
the
full tests needed
|
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. |
The XCom.get_value has been added in 2.3.0 and while there are cases it should be used in the providers when task are mapped, in order to keep compatibility with earlier versions of Airlfow, the XCom.get_value() should only be used when ti_key is not None. We check for the construct used in community providers automatically and also add a documentation for users who would like to use dynamic task mapping featuers in their own providers.
potiuk
force-pushed
the
add-protection-against-incompatible-xcom-getvalue-usage
branch
from
6161beb to
7421d83
Compare
|
Rebased to take advantage of the "docs" build fix |
potiuk
deleted the
add-protection-against-incompatible-xcom-getvalue-usage
branch
ephraimbuddy
added
the
type:misc/internal
The XCom.get_value has been added in 2.3.0 and while there are
cases it should be used in the providers when task are mapped,
in order to keep compatibility with earlier versions of Airlfow,
the XCom.get_value() should only be used when ti_key is not None.
We check for the construct used in community providers automatically
and also add a documentation for users who would like to use
dynamic task mapping featuers in their own providers.
^ 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.