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 task.trigger rule to grid_data #30130

Merged
merged 2 commits into from
Mar 16, 2023
Merged

Conversation

bbovenzi
Copy link
Contributor

We added task.trigger_rule to the old graph view in 2.4.0 (see #26043). But that wasn't in the grid view nor in the new graph view.

Now it is included in the tooltip and in the instance details panel.

Screenshot 2023-03-15 at 12 01 47 AM

Screenshot 2023-03-15 at 12 01 07 AM


^ 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.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Mar 15, 2023
@bbovenzi bbovenzi added this to the Airflow 2.5.3 milestone Mar 15, 2023
@@ -297,7 +297,7 @@ def dag_to_grid(dag: DagModel, dag_runs: Sequence[DagRun], session: Session):
grouped_tis = {task_id: list(tis) for task_id, tis in itertools.groupby(query, key=lambda ti: ti.task_id)}

def task_group_to_grid(item, grouped_tis, *, is_parent_mapped: bool):
if isinstance(item, AbstractOperator):
if not isinstance(item, TaskGroup):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

trigger_rule doesn't exist on AbstractOperator. Open to ideas on a better way to check if this is a task or a task group.

Copy link
Member

Choose a reason for hiding this comment

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

This seems okay to me.

Comment on lines +43 to +48
group={{
id: "task",
label: "task",
instances: [],
triggerRule: "all_failed",
}}
Copy link
Member

Choose a reason for hiding this comment

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

This feels wrong. A "group" doesn't have a trigger rule. Should this be "node" or "item", not "group"? Definitely a problem for another PR, but we should probably rename this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I already started a separate branch to make our names and types consistent.

@@ -297,7 +297,7 @@ def dag_to_grid(dag: DagModel, dag_runs: Sequence[DagRun], session: Session):
grouped_tis = {task_id: list(tis) for task_id, tis in itertools.groupby(query, key=lambda ti: ti.task_id)}

def task_group_to_grid(item, grouped_tis, *, is_parent_mapped: bool):
if isinstance(item, AbstractOperator):
if not isinstance(item, TaskGroup):
Copy link
Member

Choose a reason for hiding this comment

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

This seems okay to me.

@bbovenzi bbovenzi merged commit 34e1f31 into apache:main Mar 16, 2023
@bbovenzi bbovenzi deleted the grid-trigger-rule branch March 16, 2023 15:22
@pierrejeambrun pierrejeambrun added the type:improvement Changelog: Improvements label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants