Skip to content

Move bundle selection types and EreportFilters to nexus-types#10089

Open
mergeconflict wants to merge 1 commit intomainfrom
mergeconflict/fm-sb-type-relocation
Open

Move bundle selection types and EreportFilters to nexus-types#10089
mergeconflict wants to merge 1 commit intomainfrom
mergeconflict/fm-sb-type-relocation

Conversation

@mergeconflict
Copy link
Contributor

@mergeconflict mergeconflict commented Mar 18, 2026

Move:

  • BundleDataSelection (from omicron_nexus::app::background::tasks::support_bundle::request), along with its constituent bits: BundleDataCategory, BundleData, SledSelection, into nexus_types::support_bundle
  • EreportFilters (from nexus_db_queries::db::datastore::ereport), used by BundleData::Ereports, into nexus_types::fm::ereport.

... so they can be shared between the support bundle collector (which currently uses them) and fault management's various Case types (in PR #10090).

This is a pure refactor, no behavior changes here.

@mergeconflict mergeconflict marked this pull request as draft March 18, 2026 22:21
@mergeconflict mergeconflict force-pushed the mergeconflict/fm-sb-type-relocation branch 3 times, most recently from 08b5f84 to d87449e Compare March 18, 2026 23:14
@mergeconflict mergeconflict requested review from hawkw and smklein March 18, 2026 23:21
@mergeconflict mergeconflict marked this pull request as ready for review March 18, 2026 23:26
@mergeconflict mergeconflict force-pushed the mergeconflict/fm-sb-type-relocation branch from d87449e to 7a5224c Compare March 19, 2026 00:00
@mergeconflict mergeconflict requested a review from hawkw March 19, 2026 00:14
}

impl EreportFilters {
pub fn check_time_range(&self) -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me wants to make the fields private and do the validation in a constructor, so all instances of the type are validated...but, looking back, I think the reason I didn't do it that way originally is that I wanted the start and end timestamps to be provided as named fields, rather than as unnamed function parameters, so it was obvious to the reader which is which. Given that, I don't really think it's worth messing with.

If you really wanted to do it the Right Way, you could have the constructor take one of the std range types as a single argument, so you could call it with start...end or start... or ...end and it would always be obvious to the reader. But let's treat that as a fun side quest you can do if you want, rather than something I care deeply about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that too, actually. But I figure I should probably do that in a separate PR just to make it super obvious that I'm purely moving things here.

Move BundleDataCategory, BundleData, BundleDataSelection, and SledSelection
to nexus/types/src/support_bundle.rs. Move EreportFilters to
nexus/types/src/fm/ereport.rs (where it belongs alongside other ereport
types). Update all consumers to import directly.

Pure refactor, no behavior change.
@mergeconflict mergeconflict force-pushed the mergeconflict/fm-sb-type-relocation branch from 7a5224c to 3a6ea71 Compare March 19, 2026 02:51
@mergeconflict mergeconflict self-assigned this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants