-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
Filter XCOM by key when calculating map lengths #24530
Filter XCOM by key when calculating map lengths #24530
Conversation
MappedOperator determines how many downstream task instances to create by counting XCOMs associated with upstream mapped tasks. The current implementation assumes each upstream mapped task has a single XCOM. When upstream tasks push additional XCOMs beyond the default one then MappedOperator will generate too many downstream task instances. This change updates the XCOM counting query to only consider XCOMs with a key matching the XCOM_RETURN_KEY constant.
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
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.
Change looks good - can you add to the tests please?
Sure, I'll put a test or two in |
@hinnefe2, gentle ping. It'd be great to get this into 2.3.3. That test file is the right spot. If you have further questions/get stuck, feel free to ping us in #development on Slack! |
Thanks for the nudge. I'm having a little trouble getting the test to work as intended but I'll sort that out on Slack. |
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 styling nitpick, the logic looks good to me.
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 pushed a couple of commits to resolve conflicts and update the test.
I’m going to make the call since this is straightfoward enough. |
Awesome work, congrats on your first merged pull request! |
🎉 Thanks for the help getting this across the finish line! |
MappedOperator determines how many downstream task instances to create
by counting XCOMs associated with upstream mapped tasks. The current
implementation assumes each upstream mapped task has a single XCOM. When
upstream tasks push additional XCOMs beyond the default one then
MappedOperator will generate too many downstream task instances. This
change updates the XCOM counting query to only consider XCOMs with a key
matching the
XCOM_RETURN_KEY
constant.close #24487
close #23792