-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Remove DAG refresh buttons #17263
Remove DAG refresh buttons #17263
Conversation
Sorry, I don't quite understand. Why does permission syncing remove the need for a manual refresh? |
The refresh button used to force the webserver to go re-read the dag from the file on disk (and as a result also sync the per-dag permissions.) But since Airflow 2+ the webserver only looks at the serialized copy and never read the file on disk, so the last remaining thing the refresh button did was update the per-dag permissions defined on the |
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.
The endpoints in the view are not part of either published API, so we can just remove them directly I think.
Makes sense! I didn't see these endpoints being used anywhere else so yeah I agree we can remove them entirely. Not in this PR, but we may want to reevaluate the action buttons now that there are only two of them. |
I'm good with that. @kaxil and I were just trying to be extra careful even if it wasn't part of the published API. Seems there is consensus to remove them though 👍 |
❤️ to remove stuff :) |
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
Now that the DAG parser syncs DAG specific permissions there really isn't a need to manually refresh DAGs via the UI.
871f0b4
to
4d1deab
Compare
Now that the DAG parser syncs DAG specific permissions there really
isn't a need to manually refresh DAGs via the UI.
related: #17166