villebro commented on code in PR #20632:
URL: https://github.com/apache/superset/pull/20632#discussion_r918560508
##########
superset/key_value/utils.py:
##########
@@ -63,3 +64,9 @@ def get_uuid_namespace(seed: str) -> UUID:
md5_obj = md5()
md5_obj.update(seed.encode("utf-8"))
return UUID(md5_obj.hexdigest())
+
+
+def get_deterministic_uuid(namespace: str, payload: Any) -> UUID:
+ """Get a deterministic UUID (uuid3) from a salt and a payload."""
+ payload_str = json_dumps_w_dates(payload, sort_keys=True)
Review Comment:
@ktmud not all `KeyValue` implementations will necessarily be JSON
serializable. For instance, the `SupersetMetastoreCache` KeyValue objects
aren't JSON serialisable. While pickle isn't necessarily perfect in all
scenarios, IMO it's the most generally supported serialising/deserializing
utility for Python, and as such seems like the best solution for a general
purpose key-value store.
At the time this feature was designed there was really no requirement for
analysing the contents of the value payload, so this wasn't considered. But if
this is needed I don't see any reason why we couldn't make the serializer and
deserialiser customizable to make it possible to use another library for
encoding/decoding the values and then swapping it out for `json.dumps` and
`json.loads` for the permalink implementations (would of course require a
migration).
In the case of the proposed function, I'd recommend adding a comment in the
doctoring that the payload needs to be JSON serializable.
--
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]
