john-bodley commented on code in PR #24488:
URL: https://github.com/apache/superset/pull/24488#discussion_r1240114939
##########
superset/daos/dataset.py:
##########
@@ -361,25 +362,24 @@ def create_metric(
"""
return DatasetMetricDAO.create(properties, commit=commit)
- @staticmethod
- def bulk_delete(models: Optional[list[SqlaTable]], commit: bool = True) ->
None:
+ @classmethod
+ def bulk_delete(
+ cls, models: Optional[list[SqlaTable]], commit: bool = True
+ ) -> None:
item_ids = [model.id for model in models] if models else []
- # bulk delete, first delete related data
- if models:
- for model in models:
- model.owners = []
- db.session.merge(model)
-
db.session.query(SqlMetric).filter(SqlMetric.table_id.in_(item_ids)).delete(
- synchronize_session="fetch"
- )
- db.session.query(TableColumn).filter(
- TableColumn.table_id.in_(item_ids)
- ).delete(synchronize_session="fetch")
# bulk delete itself
try:
db.session.query(SqlaTable).filter(SqlaTable.id.in_(item_ids)).delete(
synchronize_session="fetch"
)
+
+ if models:
+ connection = db.session.connection()
+ mapper = next(iter(cls.model_cls.registry.mappers)) # type:
ignore
+
+ for model in models:
+ security_manager.dataset_after_delete(mapper, connection,
model)
Review Comment:
@craig-rueda could you help educate me as to why this is? Granted that the
DAO is only abstraction from the database (and not the security manager which
in turns interfaces with the database), but isn't this the interface we expect
commands, APIs, etc. to interface with? Note this logic would be handled by the
`after_delete` event listener if we were using the ORM, as opposed to bulk
queries, to delete the assets.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]