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

Less hacky double-rendering prevention in mapped task #25924

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

uranusjr
Copy link
Member

Instead of inventing a separate way to track what XComArg has been rendered (due to task unmapping), we can actually track this with seen_oids. This makes the mechanism considerably less hacky.

@uranusjr uranusjr changed the title Refactor _expand_mapped_kwargs to disallow None Less hacky double-rendering prevention in mapped task Aug 23, 2022
Instead of inventing a separate way to track what XComArg has been
rendered (due to task unmapping), we can actually track this with
seen_oids. This makes the mechanism considerably less hacky.
@uranusjr uranusjr force-pushed the less-hacky-double-rendering-prevention branch from f785d85 to c68bbe5 Compare August 23, 2022 23:28
@uranusjr uranusjr marked this pull request as ready for review August 24, 2022 01:08
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

It does look quite a bit less magical.

I do not follow all aspects there and cannot easily go through all scenarios, but this change looks very solid and seems to keep all the properties of the previous approach in much cleaner way, so LGTM.

@ashb ashb merged commit 187e8d6 into apache:main Aug 26, 2022
@ashb ashb deleted the less-hacky-double-rendering-prevention branch August 26, 2022 20:35
@jedcunningham jedcunningham added the type:improvement Changelog: Improvements label Sep 12, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.0 milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants