-
Notifications
You must be signed in to change notification settings - Fork 15.4k
feat: implement HITL shared links functionality #53189
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.
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
Conversation
eb81ae8
to
e7cdb92
Compare
61b93c6
to
5c1c3e8
Compare
There was a problem hiding this 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!
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
raise HTTPException( | ||
status.HTTP_400_BAD_REQUEST, | ||
str(e), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about having common functions in service
module for
create_hitl_share_link
andcreate_mapped_hitl_share_link
get_hitl_share_link
andget_mapped_ti_hitl_share_link
execute_hitl_share_link_action
andexecute_mapped_ti_hitl_share_link_action
the logic for simple one and mapped one seem the same.
Only the parameters of the routes are different.
@sunank200 i noticed in the API design you don't have try number in there. Shouldn't it require an approval every try? Moreover, why aren't we just using the uuid so we can easily uniquely identify the TI-try. |
5c1c3e8
to
de74e16
Compare
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py
Outdated
Show resolved
Hide resolved
5f844ed
to
01a1285
Compare
01a1285
to
cd43358
Compare
That is right. I have changed the logic now. |
47a4106
to
1cb57e8
Compare
I think we'll need to rebase this from main before the next review round. Thanks! |
9a6e3ef
to
99c62cd
Compare
Fix the CI feat: implement HITL shared links functionality feat: implement HITL shared links functionality # Conflicts: # airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml # airflow-core/src/airflow/api_fastapi/core_api/routes/public/hitl.py # airflow-core/src/airflow/ui/openapi-gen/requests/schemas.gen.ts # airflow-core/src/airflow/ui/openapi-gen/requests/types.gen.ts # airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_hitl.py # airflow-ctl/src/airflowctl/api/datamodels/generated.py
99c62cd
to
8a66849
Compare
@Lee-W I will create a new PR as the issue was updated and a lot of things have changed in the main. It will be easier that way with fresh PR. |
Implements HITL shared links functionality as specified in AIP-90, enabling external users to approve/reject tasks via secure links with TTL.
What's Changed
HITLSharedLinkManager
with HMAC token generation/verificationPOST /api/v2/hitl-details/share-link/{dag_id}/{dag_run_id}/{task_id}
- Create shared linksGET /api/v2/hitl-details/share-link/{dag_id}/{dag_run_id}/{task_id}
- Redirect links (UI dialog)POST /api/v2/hitl-details/share-link/{dag_id}/{dag_run_id}/{task_id}/action
- Action links (direct execution)/{map_index}
suffix[api] hitl_enable_shared_links
(default:False
),hitl_shared_link_expiration_hours
(default:24
)Testing:
The script will:
closes:#52202
^ 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.