Vitor-Avila commented on code in PR #34993:
URL: https://github.com/apache/superset/pull/34993#discussion_r2324052705
##########
superset/commands/dataset/update.py:
##########
@@ -175,8 +176,17 @@ def _validate_semantics(self, exceptions:
list[ValidationError]) -> None:
self._validate_metrics(metrics, exceptions)
if folders := self._properties.get("folders"):
+ valid_uuids: set[UUID] = set()
+ if metrics:
+ valid_uuids.update(UUID(metric["uuid"]) for metric in metrics)
Review Comment:
@betodealmeida def not a blocking comment, but the `DatasetColumnsPut` and
`DatasetMetricsPut` schemas don't have `uuid` as a required field. Do you know
if these are automatically generated by the backend during the DAO update? I
wonder if we could face an issue in which the user is for example adding 3
metrics, but specifying a `uuid` only for the metric they're using in a folder.
I think we have 3 possible options:
1. Leave it as is. Honestly, I don't think this would be something that
would cause frequent issues, could be just some edge cases.
2. If the backend is indeed generating the `uuid` value for new items on the
fly, we could move this `uuid` generation to `_validate_metrics` and
`_validate_columns` (if not present) so that by the time we get here every item
has a `uuid`.
3. Edit the schema to make these required. We would need to make sure that
the client always generate this ahead of the `PUT` call, and it could cause
issue for existing users that using the API without this field.
Let me know your thoughts
##########
superset/commands/dataset/update.py:
##########
@@ -175,8 +176,17 @@
self._validate_metrics(metrics, exceptions)
if folders := self._properties.get("folders"):
+ valid_uuids: set[UUID] = set()
+ if metrics:
+ valid_uuids.update(UUID(metric["uuid"]) for metric in metrics)
+ else:
+ valid_uuids.update(metric.uuid for metric in
self._model.metrics)
+ if columns:
+ valid_uuids.update(UUID(column["uuid"]) for column in columns)
Review Comment:
Covered this in my previous comment
--
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]
