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: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org

Reply via email to