[30/n] [omdb] move inventory collection display to nexus-types #8613
Conversation
Created using spr 1.3.6-beta.1
| if self.include_sleds { | ||
| display_sleds(&self.collection, f)?; | ||
| } | ||
| if self.include_orphaned_datasets { | ||
| display_orphaned_datasets(&self.collection, f)?; | ||
| } |
There was a problem hiding this comment.
I think this is a behavior change we don't want? The old code was:
if filter.include_sleds() {
inv_collection_print_sleds(&collection);
} else if matches!(filter, CollectionsShowFilter::OrphanedDatasets) {
inv_collection_print_orphaned_datasets(&collection);
}
If we print sleds, that already includes the orphaned datasets for each sled. CollectionsShowFilter::OrphanedDatasets was a way to say "I only want to see the orphaned datasets from each sled, not all the other details", because we want to be able to do this during the R16 upgrade as a check for any unexpected orphans without all the noise of the normal sled details.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ahh gotcha, will update and explain in a comment.
Created using spr 1.3.6-beta.1
|
|
||
| fn include_orphaned_datasets(&self) -> bool { | ||
| match self { | ||
| Self::All => true, |
There was a problem hiding this comment.
Should this be false? Although I think it maybe doesn't matter in practice thanks to the else if?
There was a problem hiding this comment.
This is what I tried first, and I thought it was more confusing for "All" to return false here than for us to use else. Logically we are including orphaned datasets, it's just part of the sled config.
sunshowers
deleted the
sunshowers/spr/30n-omdb-move-inventory-collection-display-to-nexus-types
branch
Use the display logic implemented in #8613 to show inventory collections in reconfigurator-cli. I had to change some of the timestamps to be in RFC 3339 format to allow redactions to work properly. Kind of addresses #8611, in the sense that we now have tests that render collection output and ensure it looks reasonable.
There are no major code changes in this PR -- it's pure movement + replacing
println!calls withwriteln!.There's one minor change, which is that the warning about errors now goes to stdout. I think it's okay.
Tests are coming in the next PR as part of adding support to reconfigurator-cli.
Part of #8611.