pierrejeambrun commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2161635930
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -460,24 +467,26 @@ def get_task_instances(
)
query = query.where(TI.run_id == dag_run_id)
+ filters = [
+ run_after_range,
+ logical_date_range,
+ start_date_range,
+ end_date_range,
+ update_at_range,
+ duration_range,
+ state,
+ pool,
+ queue,
+ executor,
+ version_number,
+ ]
+ if state.value is not None and None in state.value:
+ current_time = func.now()
+ for f in (start_date_range, end_date_range):
+ f.attribute = func.coalesce(f.attribute, current_time)
task_instance_select, total_entries = paginated_select(
statement=query,
- filters=[
- run_after_range,
- logical_date_range,
- start_date_range,
- end_date_range,
- update_at_range,
- duration_range,
- state,
- pool,
- queue,
- executor,
- task_id,
- task_display_name_pattern,
- version_number,
- readable_ti_filter,
- ],
+ filters=cast("Sequence[OrmClause[Any]]", filters),
Review Comment:
That would also remove the need for this I believe.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -460,24 +467,26 @@ def get_task_instances(
)
query = query.where(TI.run_id == dag_run_id)
+ filters = [
+ run_after_range,
+ logical_date_range,
+ start_date_range,
+ end_date_range,
+ update_at_range,
+ duration_range,
+ state,
+ pool,
+ queue,
+ executor,
+ version_number,
+ ]
+ if state.value is not None and None in state.value:
+ current_time = func.now()
+ for f in (start_date_range, end_date_range):
+ f.attribute = func.coalesce(f.attribute, current_time)
Review Comment:
I think we should always do this piece of code, independently or not if
`None` is in the filtering options or not.
There are other states that can have start_date or end_date equal to None,
and they would be directly excluded from any request that specifies
`start_date_range` or `end_date_range`.
Maybe we can move that logic directly inside the `start_date_range` and
`end_date_range` filter, so there is no code duplication and less complexity.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -964,6 +964,19 @@ class TestGetTaskInstances(TestTaskInstanceEndpoint):
3,
id="test state filter",
),
+ pytest.param(
+ [
+ {"state": State.RUNNING},
+ {"state": State.QUEUED},
+ {"state": State.SUCCESS},
+ {"state": State.NONE},
+ ],
+ False,
+
("/dags/example_python_operator/dagRuns/TEST_DAG_RUN_ID/taskInstances"),
+ {"state": ["no_status"]},
+ 1,
+ id="test no_status state filter",
+ ),
Review Comment:
Also we need a test with `start_date_gte` than `now-1 day` for instance and
confirm that the coalescing part is operating as expected and returning the no
status TI that has a `start_date=None` because now >= now - 1 day
--
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]
