Skip to content

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Nov 7, 2025


^ 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.

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Looks ok - BUT - this is touching the "stable" API. Even though 99% used only for UI, is this a breaking change where we need to keep the unused param for compatibility? How is you are using an "old" client with generated code against the previous API spec? The call would fail with excess parameters? Or will excess parameters be accepted?

@vincbeck
Copy link
Contributor Author

vincbeck commented Nov 7, 2025

Looks ok - BUT - this is touching the "stable" API. Even though 99% used only for UI, is this a breaking change where we need to keep the unused param for compatibility? How is you are using an "old" client with generated code against the previous API spec? The call would fail with excess parameters? Or will excess parameters be accepted?

That's a good call but this parameter is also not used so ... I do not know. Let's hear what others think

@potiuk
Copy link
Member

potiuk commented Nov 8, 2025

That's a good call but this parameter is also not used so ... I do not know. Let's hear what others think

I think extra parameters specified in URL are simply ignored by FastAPI (to be checked) so technically speaking this is not a breaking change.

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.

Thanks for the cleanup!

I think extra parameters specified in URL are simply ignored by FastAPI (to be checked) so technically speaking this is not a breaking change.

Yes, this PR isn't a breaking change IMO.

Actually #56633 is kind of the breaking change for user because we replace the frontend side Cookie with HTTP-only Cookie. ( Some contributors reported that they need to clear the browser cache after #56633 had merged, but it should not be a big problem for users)

@vincbeck vincbeck merged commit 4a35fc9 into apache:main Nov 24, 2025
118 checks passed
@vincbeck vincbeck deleted the vincbeck/unsued branch November 24, 2025 17:00
Copilot AI pushed a commit to jason810496/airflow that referenced this pull request Dec 5, 2025
itayweb pushed a commit to itayweb/airflow that referenced this pull request Dec 6, 2025
itayweb pushed a commit to itayweb/airflow that referenced this pull request Dec 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants