HsiuChuanHsu commented on code in PR #54528:
URL: https://github.com/apache/airflow/pull/54528#discussion_r2291261543
##########
airflow-core/src/airflow/api/common/delete_dag.py:
##########
@@ -88,4 +88,107 @@ def delete_dag(dag_id: str, keep_records_in_log: bool =
True, session: Session =
.execution_options(synchronize_session="fetch")
)
+ # Clean up DAG-specific permissions from Flask-AppBuilder tables
+ _cleanup_dag_permissions(dag_id, session)
+
return count
+
+
+def _cleanup_dag_permissions(dag_id: str, session: Session) -> None:
+ """
+ Clean up DAG-specific permissions from Flask-AppBuilder tables.
+
+ When a DAG is deleted, we need to clean up the corresponding permissions
+ to prevent orphaned entries in the ab_view_menu table.
+
+ This addresses issue #50905: Deleted DAGs not removed from ab_view_menu
table
+ and show up in permissions.
+ """
+ from airflow.configuration import conf
+
+ if "FabAuthManager" not in conf.get("core", "auth_manager"):
+ return
+
+ # Try to import FAB models with version compatibility
+ def _get_fab_models():
+ """Get FAB models with version compatibility handling."""
+ try:
+ from airflow.providers.fab.auth_manager import models as fab_models
+
+ return fab_models
+ except ImportError:
+ try:
+ # Handle Pre-airflow 2.9 case where FAB was part of the core
airflow
+ from airflow.providers.fab.auth.managers.fab import models as
fab_models
+
+ return fab_models
+ except ImportError:
+ # If FAB provider is not available, skip cleanup
Review Comment:
Thanks for the feedback! That makes a lot of sense.
Offloading this to the providers would make the core code a lot cleaner and
easier to maintain. I'll go ahead and implement it this way:
1. I'll create a new module, `dag_permissions.py`, to specifically handle
the DAG permission cleanup logic within the FAB provider.
2. I'll add a `cleanup_dag_permissions()` method to the `FabAuthManager`
class that delegates the task to the new module.
https://github.com/apache/airflow/blob/e00777aa13556b3a81bec00dab50bcfbf98a3d86/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py#L181
3. I'll update `delete_dag.py` to calls the `cleanup_dag_permissions()`
method on the current authentication manager.
This feels like a much better approach. I'll get started on this now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
