Skip to content
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

Update taskinstance clear endpoint to support mapped tasks #44867

Open
1 task done
eladkal opened this issue Dec 12, 2024 · 7 comments
Open
1 task done

Update taskinstance clear endpoint to support mapped tasks #44867

eladkal opened this issue Dec 12, 2024 · 7 comments
Assignees

Comments

@eladkal
Copy link
Contributor

eladkal commented Dec 12, 2024

Body

We are missing clear endpoint in the taskinstance api

We need the endpoint to have same functionality as clear button in the UI.
Note: The task needs to accommodate both clearing a task and clearing a map index (when using dynamic task mapping)

for reference you can see previous PRs that added functionality to the Rest API: #41017, #39138

Update (after discussing in comments)

  1. Move clear endpoint from Dag to TaskInstance
  2. Add support for mapped index

Committer

  • I acknowledge that I am a maintainer/committer of the Apache Airflow project.
@eladkal eladkal added kind:feature Feature Requests area:API Airflow's REST/HTTP API good first issue area:core labels Dec 12, 2024
@vatsrahul1001
Copy link
Collaborator

I can work on this. Assigning it to myself.

@vatsrahul1001 vatsrahul1001 self-assigned this Dec 12, 2024
@vatsrahul1001
Copy link
Collaborator

@eladkal Should I also create an Issue to migrate this to FastAPI as well?

@eladkal
Copy link
Contributor Author

eladkal commented Dec 12, 2024

No need for another issue. You can simply start another PR directly

@vatsrahul1001
Copy link
Collaborator

@eladkal We do have taskinstance clear endpoint implementation already here. As per openapi spec its under DAG endpoint here.

image

In FastAPI we have correctly added this under taskinstances.
image

Do let me know if this is what you wanted in implementation, if yes should I move this endpoint from DAG to TaskInstance in legacy API?

@eladkal
Copy link
Contributor Author

eladkal commented Dec 17, 2024

Do let me know if this is what you wanted in implementation, if yes should I move this endpoint from DAG to TaskInstance in legacy API?

In my perspective yes. I expect to find clear action on task instance endpoints but I think it's a good idea to track the PR added it and see if there was discussion around why it's placed there. We may be missing something.

By the way, it seems like with current endpoint you are not able to clear a specific map index in a specific task instance so I think we need to add support for this.

@pierrejeambrun
Copy link
Member

Maybe we should close this one, or update it in favor of a feature request for a more fined grained clearing capabilities on 'mapped task instances' ?

As I understand the endpoint exists, it just needs improvement ?

@eladkal
Copy link
Contributor Author

eladkal commented Dec 18, 2024

Maybe we should close this one, or update it in favor of a feature request

In the wrong place. Its not intuitive to find it under DAGs. So this issue is about moving it and refactor it to support mapped tasks. @vatsrahul1001 do you need me to update issue description?

Updated title and description

@eladkal eladkal changed the title Add taskinstance clear endpoint to the rest api Update taskinstance clear endpoint to support mapped tasks Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants