feat: Adding support for IB partition update#475
feat: Adding support for IB partition update#475hwadekar-nv wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
Matthias247
left a comment
Matthias247
left a comment
There was a problem hiding this comment.
Looks good. But can you please add a unit-test for it?
| // validate the metadata | ||
| metadata.validate(true).map_err(CarbideError::from)?; | ||
|
|
||
| if metadata.pkey != partition.metadata.pkey { |
There was a problem hiding this comment.
I'm surprised pkey is metadata. It should not be supplied by the tenant at all (just status)
There was a problem hiding this comment.
No, this has been a mistake here. pkey is under config only. -- https://github.com/NVIDIA/bare-metal-manager-core/blob/main/crates/rpc/proto/forge.proto#L1481
| } | ||
|
|
||
| // Update the metadata of the partition | ||
| partition.metadata.name = name; |
There was a problem hiding this comment.
Looks like none of this is really compiling (there's a bunch of missing variables, plus this method isn't even called by the impl Forge for Api in api.rs) but when you get around to fixing this, you should be able to just do partition.metadata = metadata.try_into()? here.
There was a problem hiding this comment.
Yes @kensimon this PR is in draft, still updating step by step
4 participants
Description
Type of Change
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes