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

BaseBranchOperator will push to xcom by default. #13763

Merged

Conversation

ashmeet13
Copy link
Contributor

This change will allow BaseBranchOperator to xcom push the branch it chose to follow.
It will also add support to use the do_xcom_push parameter.

The change includes the returning the result of choose_branch() whenever execute() is called.

Currently the BaseBranchOperator neither pushes the chosen branch to xcom nor support's the do_xcom_push parameter.

Closes: #13704

This change will BaseBranchOperator to do xcom push of the branch it choose to follow.
It will also add support to use the do_xcom_push parameter.

The added change returns the result received by running choose_branch().
self.skip_all_except(context['ti'], self.choose_branch(context))
branches_to_execute = self.choose_branch(context)
self.skip_all_except(context['ti'], branches_to_execute)
return branches_to_execute
Copy link
Member

Choose a reason for hiding this comment

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

This means it will always push right, even if do_xcom_push is False

Copy link
Contributor Author

@ashmeet13 ashmeet13 Jan 19, 2021

Choose a reason for hiding this comment

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

Not sure about this. I dont think it would always push.
I did run the test case that I added for checking xcom push (Permalink to the test function) with do_xcom_push=False and the value returned by - ti.xcom_pull(task_ids='make_choice') was None

Have I understood the issue wrong? I followed PythonBranchOperator as a guide to figure out what was needed to be done.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes,

# If the task returns a result, push an XCom containing it
if task_copy.do_xcom_push and result is not None:
self.xcom_push(key=XCOM_RETURN_KEY, value=result)
return result

should take care of it

@kaxil kaxil added this to the Airflow 2.0.1 milestone Jan 21, 2021
@kaxil kaxil merged commit 3e25795 into apache:master Jan 21, 2021
kaxil pushed a commit that referenced this pull request Jan 21, 2021
This change will BaseBranchOperator to do xcom push of the branch it choose to follow.
It will also add support to use the do_xcom_push parameter.

The added change returns the result received by running choose_branch().

Closes: #13704

(cherry picked from commit 3e25795)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BaseBranchOperator should push to xcom by default
2 participants