uranusjr commented on code in PR #22958:
URL: https://github.com/apache/airflow/pull/22958#discussion_r850110542
##########
airflow/api/common/mark_tasks.py:
##########
@@ -118,7 +119,9 @@ def set_state(
if execution_date and not timezone.is_localized(execution_date):
raise ValueError(f"Received non-localized date {execution_date}")
- task_dags = {task.dag for task in tasks}
+ t_dags = {task.dag for task in tasks if not isinstance(task, tuple)}
+ t_dags_2 = {item[0].dag for item in tasks if isinstance(item, tuple)}
+ task_dags = t_dags | t_dags_2
Review Comment:
```suggestion
task_dags = {
task[0].dag if isinstance(task, tuple) else task.dag
for task in tasks
}
```
##########
airflow/models/dag.py:
##########
@@ -1658,9 +1679,13 @@ def set_task_instance_state(
task = self.get_task(task_id)
task.dag = self
+ if not map_indexes:
+ map_indexes = [-1]
Review Comment:
Intuitively I think not passing `map_idnexes` should default to setting
states of all task instances of this task, not just -1 (matters if the task is
mapped).
Another thing to consider is that, since this function is named
`set_task_instance_state` (singular), it probably should only be able to set
state to one task instance, i.e. it should only take one map index, not
multiple. If we want to allow setting multiple mapped task instances, we should
name the function `set_task_state` and deprecate this name.
##########
airflow/models/dag.py:
##########
@@ -1368,6 +1370,7 @@ def _get_task_instances(
self,
*,
task_ids,
+ task_ids_and_map_indexes,
Review Comment:
Maybe merge these like `set_states`? (same for other arguments below,
including the `exclude` ones)
Also maybe add type annotations to these, it’s not easy to tell what should
be passed into these at first sight.
##########
airflow/www/views.py:
##########
@@ -2025,6 +2028,14 @@ def clear(self):
origin = get_safe_url(request.form.get('origin'))
dag = current_app.dag_bag.get_dag(dag_id)
+ map_indexes = request.form.get('map_indexes')
Review Comment:
There’s `getlist` available for this. To use it we need to format the
argument list in the front end in a specific way, but that’s better than
manually parsing the list, I think.
--
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]
