jedcunningham commented on code in PR #30478:
URL: https://github.com/apache/airflow/pull/30478#discussion_r1159113758
##########
airflow/models/dag.py:
##########
@@ -1840,6 +1840,7 @@ def set_task_instance_state(
past: bool = False,
commit: bool = True,
session=NEW_SESSION,
+ group_id: str | None = None,
Review Comment:
I'm not sure I like the pattern of `set_task_instance_state` acting on a
task instance, or a group when you pass this optional group_id (thus, ignoring
the task_id completely). I think I'd rather see a `set_task_group_state`
instead.
##########
airflow/models/dag.py:
##########
@@ -1915,9 +1943,10 @@ def set_task_instance_state(
only_failed=True,
session=session,
# Exclude the task itself from being cleared
- exclude_task_ids={task_id},
+ exclude_task_ids=task_ids,
Review Comment:
This type isn't right? I'm not sure why this isn't failing with mypy, but
doesn't this expect a frozenset?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
