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