CP-34028: implement Uuid with Uuidm #103
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also adds a pretty_printer and an equal function Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Change signature functions that generate UUIDs from inputs to return a UUID as an option. They now transform the input to make sure the resulting string is a valid UUID. Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Also reorganized the functions and signatures to group similar functionality together. Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
It's useful to extract uuid from hashed bytes Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
|
While having the phantom type makes sense to avoid comparing uuid of different types of resources, xapi needs to have a dedicated type to model uuids instead of using strings. On top of this, using Uuid.t means that for every roundtrip we're paying the cost of converting from the string representation to the raw byte value and viceversa which may impact performance, having the string representation with the new interface may be more desirable. |
|
Obsoleted by the xapi PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Uuid module is problematic because it doesn't parse input at all. This PR tries to do a better job at that and replaces its base type from string to Uuidm, which should make the representation more compact when used.
In order to guard from guard values it's necessary to change the interface to use Option: using exceptions would be dangerous and some uses might be missed and lead to unhandled exceptions.
Draft PR as it has a breaking change so clients need to be modified first; I would like some feedback before doing that.
TODO: The functions that generate random UUID do not use the ones provided by Uuidm. I don't think the Option.get is worrying because previously a different exception would have been raised, but there's a chance to make it more robust.