Skip to content

Merge mapped and non-mapped HITL endpoints into one #54723

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
Lee-W merged 3 commits into from
Aug 22, 2025
Merged

Merge mapped and non-mapped HITL endpoints into one #54723

Lee-W merged 3 commits into from
Aug 22, 2025

Conversation

@guan404ming
Copy link
Member

@guan404ming guan404ming commented Aug 20, 2025 •
edited
Loading

Related PR

Why

  • it would be more clear to have only one endpoint to handle mapped / non-mapped hitl ti

How

  • merge two endpoints into one could handle mapped / non-mapped hitl ti
  • update related tests
  • update frontend usage

^ 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 area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. labels Aug 20, 2025
@Lee-W
Copy link
Member

Lee-W commented Aug 20, 2025

I think we also need to update GET as well

@guan404ming
Copy link
Member Author

guan404ming commented Aug 20, 2025

I would update later, thanks!

@guan404ming guan404ming marked this pull request as draft August 20, 2025 12:18
@guan404ming guan404ming force-pushed the hitl-refactor-update branch from ad64181 to c5436b2 Compare August 20, 2025 18:22
@guan404ming guan404ming changed the title Refactor updateHitlDetail endpoint Refactor hitl public endpoint Aug 20, 2025
@guan404ming guan404ming force-pushed the hitl-refactor-update branch from c5436b2 to 2de6687 Compare August 20, 2025 19:03
@guan404ming guan404ming marked this pull request as ready for review August 20, 2025 19:03
@guan404ming
Copy link
Member Author

guan404ming commented Aug 20, 2025

Updated.🫡

Lee-W
Lee-W reviewed Aug 21, 2025
View reviewed changes
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

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

I think we no longer need

airflow/airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py

Lines 86 to 90 in 0204a89

if map_index is None and task_instance.map_index != -1:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail="Task instance is mapped, add the map_index value to the URL",
)
.

some nits from backend side but mostly looks great

@guan404ming guan404ming force-pushed the hitl-refactor-update branch from 2de6687 to 65bb405 Compare August 21, 2025 08:01
Lee-W
Lee-W reviewed Aug 21, 2025
View reviewed changes
@Lee-W Lee-W changed the title Refactor hitl public endpoint Merge mapped and non-mapped HITL endpoints into one Aug 21, 2025
@guan404ming @Lee-W
Apply suggestions from code review
dcd2e62 
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
jason810496
jason810496 approved these changes Aug 22, 2025
View reviewed changes
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the PR, LGTM overall.

It would be better to place _get_task_instance, _update_hitl_detail and _get_hitl_detail common functions under service module. Non-blocking, this can be done in follow-up PR, thanks!

@jason810496 jason810496 mentioned this pull request Aug 22, 2025
Merged
@Lee-W
Copy link
Member

Lee-W commented Aug 22, 2025

Nice! Thanks for the PR, LGTM overall.

It would be better to place _get_task_instance, _update_hitl_detail and _get_hitl_detail common functions under service module. Non-blocking, this can be done in follow-up PR, thanks!

let me do it in another follow up PR

@Lee-W Lee-W merged commit a63fc7b into apache:main Aug 22, 2025
105 checks passed
@Lee-W Lee-W mentioned this pull request Aug 22, 2025
Closed
1 task
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
@guan404ming @Lee-W @mangal-vairalkar
Merge mapped and non-mapped HITL endpoints into one (apache#54723)
1c3eba7 
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
kaxil added a commit to apache/airflow-client-python that referenced this pull request Oct 22, 2025
@kaxil
Update Python Client to 3.1.0rc1
e62c042 
(from https://github.com/apache/airflow/tree/python-client/3.1.0rc1)

## New Features:

- Add `map_index` filter to TaskInstance API queries ([#55614](apache/airflow#55614))
- Add `has_import_errors` filter to Core API GET /dags endpoint ([#54563](apache/airflow#54563))
- Add `dag_version` filter to get_dag_runs endpoint ([#54882](apache/airflow#54882))
- Implement pattern search for event log endpoint ([#55114](apache/airflow#55114))
- Add asset-based filtering support to DAG API endpoint ([#54263](apache/airflow#54263))
- Add Greater Than and Less Than range filters to DagRuns and Task Instance list ([#54302](apache/airflow#54302))
- Add `try_number` as filter to task instances ([#54695](apache/airflow#54695))
- Add filters to Browse XComs endpoint ([#54049](apache/airflow#54049))
- Add Filtering by DAG Bundle Name and Version to API routes ([#54004](apache/airflow#54004))
- Add search filter for DAG runs by triggering user name ([#53652](apache/airflow#53652))
- Enable multi sorting (AIP-84) ([#53408](apache/airflow#53408))
- Add `run_on_latest_version` support for backfill and clear operations ([#52177](apache/airflow#52177))
- Add `run_id_pattern` search for Dag Run API ([#52437](apache/airflow#52437))
- Add tracking of triggering user to Dag runs ([#51738](apache/airflow#51738))
- Expose DAG parsing duration in the API ([#54752](apache/airflow#54752))

## New API Endpoints:

- Add Human-in-the-Loop (HITL) endpoints for approval workflows ([#52868](apache/airflow#52868), [#53373](apache/airflow#53373), [#53376](apache/airflow#53376), [#53885](apache/airflow#53885), [#53923](apache/airflow#53923), [#54308](apache/airflow#54308), [#54310](apache/airflow#54310), [#54723](apache/airflow#54723), [#54773](apache/airflow#54773), [#55019](apache/airflow#55019), [#55463](apache/airflow#55463), [#55525](apache/airflow#55525), [#55535](apache/airflow#55535), [#55603](apache/airflow#55603), [#55776](apache/airflow#55776))
- Add endpoint to watch dag run until finish ([#51920](apache/airflow#51920))
- Add TI bulk actions endpoint ([#50443](apache/airflow#50443))
- Add Keycloak Refresh Token Endpoint ([#51657](apache/airflow#51657))

## Deprecations:

- Mark `DagDetailsResponse.concurrency` as deprecated ([#55150](apache/airflow#55150))

## Bug Fixes:

- Fix dag import error modal pagination ([#55719](apache/airflow#55719))
@kaxil kaxil mentioned this pull request Oct 22, 2025
Merged
kaxil added a commit to apache/airflow-client-python that referenced this pull request Oct 23, 2025
@kaxil
Update Python Client to 3.1.0rc1 (#138)
241405e 
(from https://github.com/apache/airflow/tree/python-client/3.1.0rc1)

## New Features:

- Add `map_index` filter to TaskInstance API queries ([#55614](apache/airflow#55614))
- Add `has_import_errors` filter to Core API GET /dags endpoint ([#54563](apache/airflow#54563))
- Add `dag_version` filter to get_dag_runs endpoint ([#54882](apache/airflow#54882))
- Implement pattern search for event log endpoint ([#55114](apache/airflow#55114))
- Add asset-based filtering support to DAG API endpoint ([#54263](apache/airflow#54263))
- Add Greater Than and Less Than range filters to DagRuns and Task Instance list ([#54302](apache/airflow#54302))
- Add `try_number` as filter to task instances ([#54695](apache/airflow#54695))
- Add filters to Browse XComs endpoint ([#54049](apache/airflow#54049))
- Add Filtering by DAG Bundle Name and Version to API routes ([#54004](apache/airflow#54004))
- Add search filter for DAG runs by triggering user name ([#53652](apache/airflow#53652))
- Enable multi sorting (AIP-84) ([#53408](apache/airflow#53408))
- Add `run_on_latest_version` support for backfill and clear operations ([#52177](apache/airflow#52177))
- Add `run_id_pattern` search for Dag Run API ([#52437](apache/airflow#52437))
- Add tracking of triggering user to Dag runs ([#51738](apache/airflow#51738))
- Expose DAG parsing duration in the API ([#54752](apache/airflow#54752))

## New API Endpoints:

- Add Human-in-the-Loop (HITL) endpoints for approval workflows ([#52868](apache/airflow#52868), [#53373](apache/airflow#53373), [#53376](apache/airflow#53376), [#53885](apache/airflow#53885), [#53923](apache/airflow#53923), [#54308](apache/airflow#54308), [#54310](apache/airflow#54310), [#54723](apache/airflow#54723), [#54773](apache/airflow#54773), [#55019](apache/airflow#55019), [#55463](apache/airflow#55463), [#55525](apache/airflow#55525), [#55535](apache/airflow#55535), [#55603](apache/airflow#55603), [#55776](apache/airflow#55776))
- Add endpoint to watch dag run until finish ([#51920](apache/airflow#51920))
- Add TI bulk actions endpoint ([#50443](apache/airflow#50443))
- Add Keycloak Refresh Token Endpoint ([#51657](apache/airflow#51657))

## Deprecations:

- Mark `DagDetailsResponse.concurrency` as deprecated ([#55150](apache/airflow#55150))

## Bug Fixes:

- Fix dag import error modal pagination ([#55719](apache/airflow#55719))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lee-W Lee-W Lee-W left review comments

@jason810496 jason810496 jason810496 approved these changes

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

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

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

@jscheffl jscheffl Awaiting requested review from jscheffl

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

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

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

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

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@guan404ming @Lee-W @jason810496