john-bodley commented on code in PR #24488:
URL: https://github.com/apache/superset/pull/24488#discussion_r1242937469
##########
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 I generally agree, but how does this differ from the DAO asking
"please delete these records" which triggers the SQLAlchemy ORM to handle post
deletion events (per the
[after_delete](https://docs.sqlalchemy.org/en/20/orm/events.html#sqlalchemy.orm.MapperEvents.after_delete)
event handler) which in turn invokes the security manager?
Note we typically use these SQLAlchemy events because of short comings with
either our data model and/or constructs which can't be modeled by our database
schema.
I guess what I’m saying is I would expect the DAO to behave in the same way
whether it was deleting a single entity or bulk deleting multiple entities and
explicitly calling the `affer_delete` callback (which is only invoked for the
former) ensures that the behavior is consistent.
--
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]
