Skip to content

Fix Task Instance “No Status” Filter #51880

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 24, 2025
Merged

Fix Task Instance “No Status” Filter #51880

merged 5 commits into from
Jun 24, 2025

Conversation

sunank200
Copy link
Collaborator

@sunank200 sunank200 commented Jun 18, 2025 •
edited
Loading

Previously, filtering with state=no_status against /dags/.../taskInstances returned HTTP 422 because "no_status" wasn’t a valid TaskInstanceState.

Support no_status alias in TaskInstance state filter:

  • Map no_status (and none) to SQL NULL in _transform_ti_states, and include no_status in the list of valid state values to prevent 422 errors.
  • Extend datetime_range_filter_factory so that for start_date and end_date it wraps the column in coalesce(attr, now()). This ensures rows whose dates are NULL aren’t dropped by any ≥/≤ bounds.

Added a unit test and tested it after the change:
Screenshot 2025-06-18 at 4 44 37 PM
Screenshot 2025-06-19 at 2 37 59 PM

Closes #51246

^ 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 airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label Jun 18, 2025
pierrejeambrun
pierrejeambrun reviewed Jun 18, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

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.

@sunank200
Copy link
Collaborator Author

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

@sunank200 sunank200 requested a review from pierrejeambrun June 18, 2025 16:48
@karenbraganz
Copy link
Collaborator

@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.
image

See my comment on the issue for more context.

@uranusjr
Copy link
Member

Users expect it to return tasks whose state IS NULL.

Why is this the case? Is this due to backward compatibility?

@sunank200
Copy link
Collaborator Author

sunank200 commented Jun 19, 2025 •
edited
Loading

@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. image

See my comment on the issue 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.

Tested the change here
Screenshot 2025-06-19 at 2 37 59 PM

@sunank200
Copy link
Collaborator Author

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.

pierrejeambrun
pierrejeambrun requested changes Jun 19, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@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

I am referring to the possible values of the query parameter. Now "no_status" is a possible value for this query parameter, and the documentation should reflect that.

Edit: The param spec is too wide currently state is 'list[str]' in the doc, where is should be list[TaskInstance], so indeed nothing needs to be updated. (another PR to improve the spec maybe, but unrelated)

One change required I think.

@sunank200 sunank200 force-pushed the no-status-api branch 3 times, most recently from 9a558b0 to 2c77a66 Compare June 23, 2025 12:05
@sunank200 sunank200 requested a review from pierrejeambrun June 23, 2025 13:14
@sunank200 sunank200 force-pushed the no-status-api branch 3 times, most recently from 7b4593b to 7020ac5 Compare June 23, 2025 13:26
pierrejeambrun
pierrejeambrun reviewed Jun 23, 2025
View reviewed changes
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

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

Nice small suggestions

@sunank200 sunank200 requested a review from pierrejeambrun June 23, 2025 15:41
@sunank200 sunank200 force-pushed the no-status-api branch 3 times, most recently from dd78d09 to 349c578 Compare June 23, 2025 17:10
@pierrejeambrun pierrejeambrun added this to the Airflow 3.0.3 milestone Jun 24, 2025
@pierrejeambrun pierrejeambrun added the backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch label Jun 24, 2025
@pierrejeambrun pierrejeambrun merged commit c71566b into apache:main Jun 24, 2025
100 checks passed
@pierrejeambrun pierrejeambrun deleted the no-status-api branch June 24, 2025 11:48
github-actions bot pushed a commit that referenced this pull request Jun 24, 2025
@sunank200
[v3-0-test] Fix Task Instance “No Status” Filter (#51880)
2a3ca45 
* Support no_status alias in TaskInstance state filter for REST API

* Allow 'no_status' state filter and include no_status in valid state list; skip date filters when filtering for null state

* Fix NULL-state filtering in get_mapped_task_instances by coalescing date fields

* Refactor datetime_range_filter_factory: coalesce only start_date and end_date filters

* Add a test
(cherry picked from commit c71566b)

Co-authored-by: Ankit Chaurasia <[email protected]>
@github-actions GitHub Actions
Copy link

Backport successfully created: v3-0-test

Status Branch Result
✅ v3-0-test PR Link

github-actions bot pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Jun 24, 2025
@sunank200
[v3-0-test] Fix Task Instance “No Status” Filter (apache#51880)
83826ee 
* Support no_status alias in TaskInstance state filter for REST API

* Allow 'no_status' state filter and include no_status in valid state list; skip date filters when filtering for null state

* Fix NULL-state filtering in get_mapped_task_instances by coalescing date fields

* Refactor datetime_range_filter_factory: coalesce only start_date and end_date filters

* Add a test
(cherry picked from commit c71566b)

Co-authored-by: Ankit Chaurasia <[email protected]>
pierrejeambrun pushed a commit that referenced this pull request Jun 24, 2025
@github-actions @sunank200
[v3-0-test] Fix Task Instance “No Status” Filter (#51880) (#52154)
ebbe3b7 
* Support no_status alias in TaskInstance state filter for REST API

* Allow 'no_status' state filter and include no_status in valid state list; skip date filters when filtering for null state

* Fix NULL-state filtering in get_mapped_task_instances by coalescing date fields

* Refactor datetime_range_filter_factory: coalesce only start_date and end_date filters

* Add a test
(cherry picked from commit c71566b)

Co-authored-by: Ankit Chaurasia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@rawwar rawwar Awaiting requested review from rawwar rawwar is a code owner

@jason810496 jason810496 Awaiting requested review from jason810496 jason810496 is a code owner

@bugraoz93 bugraoz93 Awaiting requested review from bugraoz93 bugraoz93 is a code owner

@shubhamraj-git shubhamraj-git Awaiting requested review from shubhamraj-git shubhamraj-git is a code owner

@uranusjr uranusjr Awaiting requested review from uranusjr

@phanikumv phanikumv Awaiting requested review from phanikumv

@karenbraganz karenbraganz Awaiting requested review from karenbraganz

Assignees
No one assigned
Labels
area:API Airflow's REST/HTTP API backport-to-v3-0-test Mark PR with this label to backport to v3-0-test branch
Projects
None yet
Milestone
Airflow 3.0.3
Development

Successfully merging this pull request may close these issues.

"No Status" task filter leads to HTTP ERROR 422
4 participants
@sunank200 @karenbraganz @uranusjr @pierrejeambrun