john-bodley commented on code in PR #24969:
URL: https://github.com/apache/superset/pull/24969#discussion_r1291892184
##########
superset/daos/dashboard.py:
##########
@@ -181,35 +180,21 @@ def validate_update_slug_uniqueness(dashboard_id: int,
slug: str | None) -> bool
return not db.session.query(dashboard_query.exists()).scalar()
return True
- @staticmethod
- def update_charts_owners(model: Dashboard, commit: bool = True) ->
Dashboard:
- owners = list(model.owners)
- for slc in model.slices:
- slc.owners = list(set(owners) | set(slc.owners))
- if commit:
- db.session.commit()
- return model
-
@classmethod
- def delete(cls, items: Dashboard | list[Dashboard], commit: bool = True)
-> None:
+ def delete(cls, items: Dashboard | list[Dashboard]) -> None:
item_ids = [item.id for item in get_iterable(items)]
- try:
+
+ with db.session.begin_nested():
db.session.query(Dashboard).filter(Dashboard.id.in_(item_ids)).delete(
synchronize_session="fetch"
)
- if commit:
- db.session.commit()
- except SQLAlchemyError as ex:
- db.session.rollback()
- raise ex
@staticmethod
- def set_dash_metadata( # pylint: disable=too-many-locals
+ def set_dash_metadata(
Review Comment:
I removed the commit in this method as I was unsure whether there was an
expectation of committing purely when setting fields. Previously this method
was called with both `commit=False` and `commit=True`, however the later was
unnecessary as it was followed up by a commit.
The TL;DR is "update*" methods still persist the data to the database
whereas `set*` do not.
--
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]
