[nexus] move UuidRng code out to a common crate, make CollectionBuilder use it #5341
Conversation
This file contains hidden or 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
Created using spr 1.3.6-beta.1
6 tasks
andrewjstone
approved these changes
Mar 28, 2024
| H: Hash, | ||
| H2: Hash, | ||
| { | ||
| // XXX: is Hash really the right thing to use here? That's what |
Contributor
There was a problem hiding this comment.
The guarantees around stability seem nice in stable-hash, especially if we want to persist these values. However, the backwards compatibility seems dangerous in that it ignores default values. Probably best to stick with this for now.
jgallagher
approved these changes
Mar 28, 2024
Created using spr 1.3.6-beta.1
sunshowers
deleted the
sunshowers/spr/nexus-move-uuidrng-code-out-to-a-common-crate-make-collectionbuilder-use-it
branch
sunshowers
added a commit
that referenced
this pull request
Mar 29, 2024
While developing #5238, I noticed that the output was getting significantly busier and less aligned. I decided to prototype out using `tabled` to display outputs, and I really liked the results. Examples that cover all of the cases are included in the PR. In the future I'd also like to add color support on the CLI, and expand it to inventory and `omdb` (it's similar except it doesn't have the zone policy table). Some other changes that are bundled into this PR: * Sort by (zone type, zone ID) rather than zone ID, to keep zones of the same type grouped together. * Moved unchanged data to the top to allow users to see less scrollback. * Moved metadata to the bottom for the same reason. * Add information about the zone config being changed. * Change `Blueprint::diff_sleds` and `Blueprint::diff_sleds_from_collection` to `Blueprint::diff_since_blueprint` and `diff_since_collection` recently. * Reordered `diff_since_blueprint`'s arguments so that `self` is after and the argument is before, to align with `diff_since_collection`. (I found that surprising!) * Renamed the diff type from `OmicronZonesDiff` to `BlueprintDiff`, since it's going to contain a lot more than zones. * Return an error from the diff methods, specifically if the before and after have the same zone ID but different types. Depends on #5238 and #5341.
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.
In #5270, we need determinism not just from blueprints but also collections. So
move the UuidRng into a common place.
As part of that, I also decided to make it its own crate and write some
documentation about it, making it more generic along the way. I think this
should be a pretty clean representation of what this is trying to do.