You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sunank200 (via GitHub)" <gi...@apache.org> on 2025/06/18 11:00:24 UTC
[PR] Fix Task Instance “No Status” Filter [airflow]
sunank200 opened a new pull request, #51880:
URL: https://github.com/apache/airflow/pull/51880
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Support no_status alias in TaskInstance state filter for REST API.
Previously, filtering with `state=no_status` against `/dags/.../taskInstances` returned `HTTP 422` because "no_status" wasn’t a valid TaskInstanceState. Users expect it to return tasks whose state IS NULL.
Added a unit test and tested it after the change:
<img width="1724" alt="Screenshot 2025-06-18 at 4 44 37 PM" src="https://github.com/user-attachments/assets/bb850a8a-ae55-489e-8750-91ee1fad44d1" />
Closes [#51246](https://github.com/apache/airflow/issues/51246)
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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 [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2161932207
##########
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:
Changed the test
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2156967914
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -194,9 +196,10 @@ def get_mapped_task_instances(
error_message = f"Task id {task_id} is not mapped"
raise HTTPException(status.HTTP_404_NOT_FOUND, error_message)
- task_instance_select, total_entries = paginated_select(
- statement=query,
- filters=[
+ if state.value is not None and None in state.value:
+ filters = [state, pool, queue, executor, version_number]
+ else:
+ filters = [
Review Comment:
I don't think we should do that, plainly ignoring the filters if `none` is in the state filtering.
What we do for the `dashboard.py` `historical_metrics` is better I think.
We coalesce the `start_date` and `end_date` for tasks that have None there (no_status, pending, etc...)
```
task_instance_states = session.execute(
select(TaskInstance.state, func.count(TaskInstance.run_id))
.join(TaskInstance.dag_run)
.where(
func.coalesce(DagRun.start_date, current_time) >= start_date,
func.coalesce(DagRun.end_date, current_time) <= func.coalesce(end_date, current_time),
)
.group_by(TaskInstance.state)
).all()
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -194,9 +196,10 @@ def get_mapped_task_instances(
error_message = f"Task id {task_id} is not mapped"
raise HTTPException(status.HTTP_404_NOT_FOUND, error_message)
- task_instance_select, total_entries = paginated_select(
- statement=query,
- filters=[
+ if state.value is not None and None in state.value:
+ filters = [state, pool, queue, executor, version_number]
+ else:
+ filters = [
Review Comment:
(Basically a task that has `None` as a start_date, will have a start_date > now, and therefore we can still include it if the filter is < now. (same logic but reversed for end date)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #51880:
URL: https://github.com/apache/airflow/pull/51880
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2161590238
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/task_instances.py:
##########
@@ -194,9 +196,10 @@ def get_mapped_task_instances(
error_message = f"Task id {task_id} is not mapped"
raise HTTPException(status.HTTP_404_NOT_FOUND, error_message)
- task_instance_select, total_entries = paginated_select(
- statement=query,
- filters=[
+ if state.value is not None and None in state.value:
+ filters = [state, pool, queue, executor, version_number]
+ else:
+ filters = [
Review Comment:
@pierrejeambrun I have fixed NULL-state filtering in get_mapped_task_instances by coalescing date fields.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-3000017087
### Backport successfully created: v3-0-test
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-0-test</td>
<td><a href="https://github.com/apache/airflow/pull/52154"><img src="https://img.shields.io/badge/PR-52154-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "karenbraganz (via GitHub)" <gi...@apache.org>.
karenbraganz commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2985908188
@sunank200 have you verified whether this properly filters for "No Status" tasks on the UI?
I had tried fixing this issue by making the same change that you made: `return [None if s in ("no_status", "none", None) else TaskInstanceState(s) for s in states]`. This prevented the Validation Error, but it did not properly filter for "No Status" task instances. I saw "No Task Instances found" even though I had a few task instances in this state.
See my [comment on the issue](https://github.com/apache/airflow/issues/51246#issuecomment-2923836210) for more context.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2161642877
##########
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 I think we need to test that the coalescing part is working as expected. Using a TI with state that is `None`, with start_date=None, and then doing a request with `start_date_gte >= now-1 day`, the TI should be returned if the coalescing operation is working
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #51880:
URL: https://github.com/apache/airflow/pull/51880#discussion_r2161909465
##########
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:
Changed it now. Please check
##########
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:
Removed it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2987332183
> @sunank200 have you verified whether this properly filters for "No Status" tasks on the UI?
>
> I had tried fixing this issue by making the same change that you made: `return [None if s in ("no_status", "none", None) else TaskInstanceState(s) for s in states]`. This prevented the Validation Error, but it did not properly filter for "No Status" task instances. I saw "No Task Instances found" even though I had a few task instances in this state. 
>
> See my [comment on the issue](https://github.com/apache/airflow/issues/51246#issuecomment-2923836210) for more context.
@karenbraganz the change currently there `return [None if s in ("no_status", "none", None) else TaskInstanceState(s) for s in states].` is needed.
But one more change is needed. In both `get_task_instances` and `get_mapped_task_instances` methods, if the incoming `state` filter includes `None`, we now *omit* all the date-range filters (run_after, start_date, etc.), since tasks with no state have no dates and would otherwise be filtered out. We only apply the non-date filters (state, pool, queue, executor, etc.) in that case. I have done the change in latest commit.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2985007569
> Nice, the openapi spec and front-end code need to be updated, because now `no_status` is also an acceptable value, which is not reflected at the moment.
@pierrejeambrun I think in case of `no_status` it is still returning None. So it won't require a change on the openapi specs or front-end code
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2987242831
> Users expect it to return tasks whose state IS NULL.
Why is this the case? Is this due to backward compatibility?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2987341295
> Why is this the case? Is this due to backward compatibility?
This is a gap between the enum-only filter and the UI’s notion of “no status.” I have added the alias for backwards compatibility with the existing UI/API contract.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Task Instance “No Status” Filter [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on PR #51880:
URL: https://github.com/apache/airflow/pull/51880#issuecomment-2987329491
> Users expect it to return tasks whose state IS NULL.
The change currently there `return [None if s in ("no_status", "none", None) else TaskInstanceState(s) for s in states].` is needed.
But one more change is needed. In both `get_task_instances` and `get_mapped_task_instances` methods, if the incoming `state` filter includes `None`, we now *omit* all the date-range filters (run_after, start_date, etc.), since tasks with no state have no dates and would otherwise be filtered out. We only apply the non-date filters (state, pool, queue, executor, etc.) in that case.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org