fix: rack_type -> rack_capability throughout#912
Conversation
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-04-12 05:55:15 UTC | Commit: 09dbb04 |
|
|
||
| [rack_types.NVL72] | ||
| [rack_types.NVL72.compute] | ||
| [rack_capabilities.NVL72] |
There was a problem hiding this comment.
@chet I am curious to use DB.
let say if we deleted /modified the a set in toml then there is hardly way to get to know about the change.
If in then we can have fk relation to Racks table to indicate the change then hence take further action.
There was a problem hiding this comment.
Yeah so in my original PR I did have that -- I would store the RackCapabilitiesSet in the database, instead of just the name of the capability for the rack. @Matthias247 had commented that, while this is something we all go back and forth over, it ends up being nicer to just reference it in the TOML file, and then if we update the TOML file, everything using that config will start to re-converge around the updated config (vs. needing to explicitly update the database column, whether it be automated or require someone to run a command).
We can always change our stance here, but for now, this is why it's like that.
|
LGTM |
09dbb04 to
6d88484
Compare
When I originally put together the mapping config in the Carbide config file, it was just a basic "rack type", but that quickly turned into `RackCapabilitiesSet`, and now that is evolving, with its own `RackHardwareType`, which is NOT a `"rack_type"`. This does a wholesale rename of `"rack_types"` -> `"rack_capabilities"` throughout, and is backwards compatible: - There's a proto field rename, but its not an index change, so it's wire compatible. - DB migration to change the column name, but everything that references it is internal, so that's fine. - I do a `serde(alias = "rack_types")` in the Carbide config so its backwards compatible in config (even though there's only a handful of dev racks using this right now). The only thing that isn't backwards compatible is `expected-racks update --rack-type`, which is now `expected-racks update --rack-capability`, BUT, again, there's only a few of us using this, and we can all quickly adopt the new naming. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
6d88484 to
6096a93
Compare
Description
When I originally put together the mapping config in the Carbide config file, it was just a basic "rack type", but that quickly turned into
RackCapabilitiesSet, and now that is evolving, with its ownRackHardwareType, which is NOT a"rack_type".This does a wholesale rename of
"rack_types"->"rack_capabilities"throughout, and is backwards compatible:serde(alias = "rack_types")in the Carbide config so its backwards compatible in config (even though there's only a handful of dev racks using this right now).The only thing that isn't backwards compatible is
expected-racks update --rack-type, which is nowexpected-racks update --rack-capability, BUT, again, there's only a few of us using this, and we can all quickly adopt the new naming.Signed-off-by: Chet Nichols III chetn@nvidia.com
Type of Change
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes