Store the UUID of the Nexus creator in Blueprints#10428
Conversation
sunshowers
left a comment
sunshowers
left a comment
There was a problem hiding this comment.
Thanks for picking this up!
| rx_blueprint, | ||
| tx_planned, | ||
| blueprint_limit: DEFAULT_BLUEPRINT_LIMIT, | ||
| creator: nexus_id.to_string(), |
There was a problem hiding this comment.
I think we'll want something like "nexus <nexus_id>" here.
There was a problem hiding this comment.
Got it, I'lll work on an updated version shortly
There was a problem hiding this comment.
Sorry, I started working on it before making sure I got this right.
You're suggesting that the string assigned to the creator struct member be the actual string "nexus <nexus_id>", right, e.g. "nexus 123e4567-e89b-12d3-a456-426614174000".
Asking, as I'm not sure to understand the value of prepending with the "nexus" string, but I might be missing something rather obvious, such as the possibility that non-nexus processes/instances could generate a blueprint.
Should we keep the member name creator as it is today, or is it better to call it planner or nexus (this one probably not a good idea if it's confirmed that it could be something other than a nexus creating it)?
There was a problem hiding this comment.
I could see going either way. I believe there are other areas (like sagas) where creator is just the Nexus uuid. We could do that here, too. It also wouldn't hurt to prepend "nexus" there for clarity. It is possible to create blueprints in other ways (e.g., reconfigurator-cli). But the blueprint's source has more information about that anyway.
There was a problem hiding this comment.
OK, I've pushed another commit c4b2614 while waiting for the comments. It uses the "nexus " format, while keeping the creator name for the struct member.
Let me know if it needs any further changes from here on!
We want blueprint.creator to store the Nexus UUID, but currently it stores a hardcoded string "blueprint_creator". This test ensures that the desired behaviour is in place.
This commit ensures that the Nexus UUID is stored in BlueprintPlanner, as reported in oxidecomputer#10396. * Add a creator element to the BlueprintPlanner structure * Store the UUID string for the Nexus creator * Update tests to ensure they either use the available nexus_id or a mock one when relevant
This will make it clear that the creator of this blueprint was a nexus instance
piffio
force-pushed
the
fix/10396-blueprint-creator-nexus-uuid
branch
from
1969528 to
c4b2614
Compare
3 participants
This is a very simple PR that replaces the hardcoded "blueprint_planner" hardcoded string with the UUID of the Nexus instance that created it as reported in #10396.
I picked it up as it's labelled as
good first issue, so I assumed it was OK to work on it as it's indeed quite simple.Please let me know if I should proceed differently in the future.
The PR contains a unit tests that ensures that the value stored is indeed the Nexus UUID.
I am not sure if it's best to call the new structure element
creatororplannerfor consistency, so any feedback in this direction would be helpful.@davepacheco, tagging you as you originally reported the issue.
Fixes #10396