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

Add documentation for task group mapping #28001

Merged
merged 4 commits into from
Dec 16, 2022

Conversation

uranusjr
Copy link
Member

I also took the chance to reorganise existing task mapping documentation a little bit to make the document flows better (for me).

Note that this is intended to be in Airflow 2.6, not 2.5.

Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Looking good!


.. note:: Arguments passed to a mapped task group is a proxy, not real values

Different from task functions (``@task``), a task group function is evaluated eagerly, and only once when the DAG is parsed. This means that parameters passed into the function is only a placeholder, and not a real value. For example, this will *not* work:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can elaborate a bit more on this. I feel like this is a important point that may be hard to understand.

Copy link
Member

Choose a reason for hiding this comment

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

Agree. The whole documentation update is great. It's hard to explain it cleared, but maybe siimply state (instead of eagerly that it is evaluated during parsing, in the scheduler and not during exection in worker?

That could help understand what's going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expanded this section a bit, hopefully it’s a bit easier to understand now.

@uranusjr uranusjr force-pushed the docs-taskgroup-expand branch from 01d9d7a to 2ac4689 Compare December 15, 2022 10:42
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.

I really love the attention to possible problems users can have with understanding those, but also what level of details it has for those who try to help the users and need to get the understanding of what are the limitations of current solution without deep diving and getting all the context that sits in the head of those who implemented it.

I think @uranusjr this is a fantastic example on how our docs should look like.

@uranusjr uranusjr merged commit 4d0fa01 into apache:main Dec 16, 2022
@uranusjr uranusjr deleted the docs-taskgroup-expand branch December 16, 2022 10:33
gschuurman pushed a commit to gschuurman/airflow that referenced this pull request Dec 19, 2022
@pierrejeambrun pierrejeambrun added this to the Airflow 2.6.0 milestone Jan 9, 2023
@pierrejeambrun pierrejeambrun added the type:doc-only Changelog: Doc Only label Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants