-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
Do not call runtime-checks api from the task sdk anymore #48125
Conversation
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.
Just 2 minor comments, looks good. Could you verify one thing though, that the API call isn't sent from the task runner anymore?
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py
Outdated
Show resolved
Hide resolved
be0246d
to
326f598
Compare
326f598
to
f8ed470
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.
Changes look good, lets wait for the CI
I'd like to check how Cadwyn handles this versioning before we merge please |
Good call! How do we test that since its an internal api? |
…versions Although right now we don't have any API clients calling the previous version since we aren't released yet and could have simply deleted the endpoint outright, this is a good opportunity to use this as a practice run for API versioning
@kaxil @amoghrajesh @pierrejeambrun Anyone else interested in the API versioning: please take a look at the second commit in this PR: 97d16b9 |
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.
re: Versioning
Reviewed, it will take some getting used too -- but look sane.
I am curious to see how things change, if an existing datamodel or its field changes and stuff. Nothing new for them -- but curious how it would look :) |
We'd already removed any meaningfull checks in this endpoint in a previous PR Although right now we don't have any API clients calling the previous version since we aren't released yet and could have simply deleted the endpoint outright, this is a good opportunity to use this as a practice run for API versioning --------- Co-authored-by: Sneha Prabhu <[email protected]> Co-authored-by: Ash Berlin-Taylor <[email protected]>
Closes: #47988
This PR removes the
/task_instances/{uuid}/runtime-checks
API and as its consumption.