Vitor-Avila commented on code in PR #32658:
URL: https://github.com/apache/superset/pull/32658#discussion_r1994328620
##########
superset/commands/database/sync_permissions.py:
##########
@@ -249,11 +249,11 @@ def _rename_database_in_permissions(
self, catalog: str | None, schemas: Iterable[str]
) -> None:
# rename existing catalog permission
+ new_catalog_perm_name = security_manager.get_catalog_perm(
+ self.db_connection.name,
+ catalog,
Review Comment:
@mistercrunch `get_catalog_perm()` supports getting called with a catalog
set to `None`, and it returns `None`:
``` python
def get_catalog_perm(
self,
database: str,
catalog: Optional[str] = None,
) -> Optional[str]:
"""
Return the database specific catalog permission.
:param database: The Superset database or database name
:param catalog: The database catalog name
:return: The database specific schema permission
"""
if catalog is None:
return None
return f"[{database}].[{catalog}]"
```
This is the expected `catalog_perm` value for datasets/charts related with a
DB connection that does not support `multi_catalog`.
We need `new_catalog_perm_name` to be set regardless if `catalog == None` or
not, because then down there (outside of this `if catalog:` block) we're doing:
``` python
...
# rename permissions on datasets and charts
for dataset in DatabaseDAO.get_datasets(
self.db_connection_id,
catalog=catalog,
schema=schema,
):
dataset.catalog_perm = new_catalog_perm_name
dataset.schema_perm = new_schema_perm_name
for chart in DatasetDAO.get_related_objects(dataset.id)["charts"]:
chart.catalog_perm = new_catalog_perm_name
chart.schema_perm = new_schema_perm_name
...
```
Currently when the code gets there we get an `UnboundLocalError` exception
as `new_catalog_perm_name` would only be declared/have a value if `catalog !=
None`.
--
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]
