-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Allow Variable and XCom set and delete from trigger code #53514
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
Allow Variable and XCom set and delete from trigger code #53514
Conversation
|
@karenbraganz could you please update the PR title to reflect what is being done? Tests are also needed in |
|
@karenbraganz how's it going with this one? |
|
@amoghrajesh I have completed writing tests that are passing for Variable set and delete. However, I am having some trouble with XCom since there is no longer a XCom.get_one() method, which I was planning to use in the test to check whether the XCom was set. I am looking into alternative ways to test whether the XCom was set. |
|
I was able to make it work with get_many() instead. |
uranusjr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m starting to wonder if it’d be a good idea to split the if-elses and maybe share duplicate parts between the triggerer and worker supervisor.
|
@uranusjr could you please elaborate? I don't completely understand what you mean by splitting if-elses and which duplicate parts you are referring to. |
|
It’s not spoecific to your PR. The trigger runner and task runner now has a lot of duplicate code handling api calls and they are increasingly difficult to manage. This probably shouldn’t be fixed in this PR but should be kept in mind. |
|
@karenbraganz I will take a look at this today, the test timeouts could be intermittent. Let me close and open this PR to trigger the tests again. |
amoghrajesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, rest looks good. Preemptively approving.
amoghrajesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good.
Backport failed to create: v3-0-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 13fa232 v3-0-testThis should apply the commit to the v3-0-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
|
Since 3.0.4 is out in RC phase, we should target this for 3.0.5. |
(cherry picked from commit 13fa232)
This PR allows the usage of Variable and XCom set & delete methods from code that is run in the triggerer.
At the moment, only Variable and XCom get methods can be used.
^ 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.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.