Skip to content

clear specific dag run TI #23516

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
potiuk merged 1 commit into from
Aug 5, 2022
Merged

clear specific dag run TI #23516

potiuk merged 1 commit into from
Aug 5, 2022

Conversation

@Bowrna
Copy link
Contributor

@Bowrna Bowrna commented May 6, 2022

closes: #23227

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 newsfragement file, named {pr_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label May 6, 2022
@Bowrna Bowrna force-pushed the clear_task_instance_api branch from 519baa9 to 94ba5f1 Compare May 7, 2022 03:47
Bowrna
Bowrna commented May 7, 2022
View reviewed changes
airflow/api_connexion/endpoints/task_instance_endpoint.py Outdated
session: Session = NEW_SESSION,
) -> APIResponse:
"""Clear task instances for given dag run."""
dag = current_app.dag_bag.get_dag(dag_id)
Copy link
Contributor Author

@Bowrna Bowrna May 7, 2022

Choose a reason for hiding this comment

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

@bbovenzi Could you tell me if the checks are ok or do i have to make any changes?

@Bowrna Bowrna force-pushed the clear_task_instance_api branch 5 times, most recently from 68bc55e to 05a202d Compare May 8, 2022 17:08
@Bowrna Bowrna marked this pull request as ready for review May 8, 2022 17:09
@Bowrna Bowrna force-pushed the clear_task_instance_api branch from 05a202d to e58efed Compare May 8, 2022 17:38
Bowrna
Bowrna commented May 8, 2022
View reviewed changes
@Bowrna Bowrna force-pushed the clear_task_instance_api branch 2 times, most recently from a5f7aa0 to b4a0180 Compare May 9, 2022 08:46
@Bowrna Bowrna force-pushed the clear_task_instance_api branch 3 times, most recently from 1ca4aeb to 77fc558 Compare June 2, 2022 10:28
@Bowrna
Copy link
Contributor Author

Bowrna commented Jun 2, 2022 •
edited
Loading

@bbovenzi @ephraimbuddy Could you share your reviews/ feedback about this PR when you get time?

Also please share if we have to include include_upstream and include_downstream param as part of this API. The existing API in views.py for /clear supports these parameters. That's why I want to know if we have to add it to this API. Thanks!

@Bowrna Bowrna force-pushed the clear_task_instance_api branch from 77fc558 to b844ed0 Compare June 3, 2022 03:50
@bbovenzi
Copy link
Contributor

bbovenzi commented Jun 3, 2022

@bbovenzi @ephraimbuddy Could you share your reviews/ feedback about this PR when you get time?

Also please share if we have to include include_upstream and include_downstream param as part of this API. The existing API in views.py for /clear supports these parameters. That's why I want to know if we have to add it to this API. Thanks!

Yes, we need those. This should act as a replacement for the current endpoint the UI uses.

@Bowrna
Copy link
Contributor Author

Bowrna commented Jun 3, 2022

Yes, we need those. This should act as a replacement for the current endpoint the UI uses.

Then I have to extract only the task_ids( excluding the map_index in tuple) and send that to query and find the upstream or downstream task_ids. Am I right in my understanding? @bbovenzi

@bbovenzi
Copy link
Contributor

bbovenzi commented Jun 9, 2022 •
edited
Loading

Then I have to extract only the task_ids( excluding the map_index in tuple) and send that to query and find the upstream or downstream task_ids. Am I right in my understanding? @bbovenzi

Yes, we'll need all of the following boolean params to send to the query:

include_upstream
include_downstream
include_future
include_past
recursive

@Bowrna Bowrna force-pushed the clear_task_instance_api branch from b844ed0 to 65ef56c Compare June 10, 2022 08:38
@Bowrna
Copy link
Contributor Author

Bowrna commented Jun 10, 2022

Yes, we'll need all of the following boolean params to send to the query:

include_upstream include_downstream include_future include_past recursive

@bbovenzi instead of using the field recursive, the existing api for clearTaskInstances have the param include_subdags and include_parentdag. The older api in views.py use the value in recursive param to fill the value for both include_subdags and include_parentdag.

Do you still think recursive field has to be given in the query param?

@bbovenzi
Copy link
Contributor

bbovenzi commented Jun 10, 2022

@bbovenzi instead of using the field recursive, the existing api for clearTaskInstances have the param include_subdags and include_parentdag. The older api in views.py use the value in recursive param to fill the value for both include_subdags and include_parentdag.

Do you still think recursive field has to be given in the query param?

Ah I didn't realize that. We don't need recursive then

@Bowrna Bowrna force-pushed the clear_task_instance_api branch 3 times, most recently from b6d2940 to d18c28b Compare June 12, 2022 09:01
@Bowrna Bowrna closed this Jun 12, 2022
@Bowrna Bowrna reopened this Jun 12, 2022
@Bowrna Bowrna force-pushed the clear_task_instance_api branch from d931df9 to 7b6656b Compare June 19, 2022 09:31
@Bowrna
Copy link
Contributor Author

Bowrna commented Jun 19, 2022

@bbovenzi @ephraimbuddy Gentle reminder.

I have added the params in v1.yaml but when I hit these params to test the API, I get an error saying Unknown field. Have anyone faced this issue? Can you tell me where I am going wrong?

@bbovenzi bbovenzi mentioned this pull request Jun 28, 2022
Closed
1 task
uranusjr
uranusjr reviewed Jul 18, 2022
View reviewed changes
uranusjr
uranusjr reviewed Jul 18, 2022
View reviewed changes
@Bowrna Bowrna force-pushed the clear_task_instance_api branch 2 times, most recently from ff7da7f to 8c29556 Compare July 26, 2022 14:20
uranusjr
uranusjr reviewed Jul 27, 2022
View reviewed changes
airflow/api_connexion/openapi/v1.yaml Outdated
description: If set to True, also tasks from past DAG Runs are affected.
type: boolean
default: false

Copy link
Member

@uranusjr uranusjr Jul 27, 2022

Choose a reason for hiding this comment

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

Also you need to add these to the Marshmallow schema (you already added dag_run_id but not the others).

@Bowrna Bowrna force-pushed the clear_task_instance_api branch 6 times, most recently from c2a3976 to bab96ba Compare August 3, 2022 03:16
@Bowrna Bowrna force-pushed the clear_task_instance_api branch from bab96ba to 2f8a025 Compare August 3, 2022 09:30
uranusjr
uranusjr reviewed Aug 3, 2022
View reviewed changes
@Bowrna Bowrna force-pushed the clear_task_instance_api branch 2 times, most recently from 1a21d3a to 5089533 Compare August 4, 2022 09:52
@Bowrna Bowrna force-pushed the clear_task_instance_api branch from 5089533 to 458a2ac Compare August 4, 2022 10:17
@Bowrna
Copy link
Contributor Author

Bowrna commented Aug 4, 2022

Could this PR be reviewed? @uranusjr @bbovenzi @ephraimbuddy

@potiuk potiuk merged commit eceb4cc into apache:main Aug 5, 2022
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 15, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.0 milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bbovenzi bbovenzi bbovenzi left review comments

@potiuk potiuk potiuk approved these changes

@uranusjr uranusjr uranusjr approved these changes

@mik-laj mik-laj Awaiting requested review from mik-laj

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton

@ashb ashb Awaiting requested review from ashb

+1 more reviewer

@josephine093 josephine093 josephine093 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API type:new-feature Changelog: New Features

Projects

None yet

Milestone

Airflow 2.4.0

Development

Successfully merging this pull request may close these issues.

Ability to clear a specific DAG Run's task instances via REST APIs

6 participants

@Bowrna @bbovenzi @potiuk @uranusjr @ephraimbuddy @josephine093