Skip to content

Conversation

@sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Jun 10, 2025

Create:

  • Inventory types with a simplified data model suitable for storing in the database
  • Database schemas, stores and loads
  • Migrations
  • Tests via the representative example

Depends on:

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@sunshowers sunshowers marked this pull request as draft June 10, 2025 21:02
@sunshowers
Copy link
Contributor Author

Likely going to re-do this based on discussion today.

@sunshowers sunshowers changed the title [13/n] send resolver status up via inventory [15/n] send resolver status up via inventory Jun 13, 2025
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
@sunshowers sunshowers marked this pull request as ready for review June 18, 2025 07:34
@sunshowers sunshowers changed the title [15/n] send resolver status up via inventory [16/n] send resolver status up via inventory Jun 18, 2025
Created using spr 1.3.6-beta.1
@sunshowers sunshowers changed the base branch from sunshowers/spr/main.13n-send-resolver-status-up-via-inventory to main June 19, 2025 20:26
Created using spr 1.3.6-beta.1
pub is_valid: bool,

/// A message describing the status. If `is_valid` is false, then this
/// message describes the reason for the invalid status.
Copy link
Contributor

Choose a reason for hiding this comment

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

What will the message describe if is_valid is true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case it's a textual representation of inventory that looks like:

5 artifacts in manifest generated by installinator (mupdate ID: 00000000-0000-0000-0000-000000000000): 5 valid, 0 mismatched, 0 errors:
  - zone1.tar.gz: valid (5 bytes, a34f924efa72cd0139ea5dc268f5d2736a9ebc858552b8f3c8cb2bed1992c31d)
  - zone2.tar.gz: valid (5 bytes, 930c5a9adbeb8410817a1dff05d7cbffe4ada2ddbeaacecadd7cbcb35a233231)
  - zone3.tar.gz: valid (5 bytes, 0f40fdb255100d0239a7e0944c8b9fdb9ba4795ad72988eaeeb00e9346236e08)
  - zone4.tar.gz: valid (5 bytes, 64d6e4a84209b4b02f4da36d24eae12b0d1ec9e414d1c3bd12a261a245148324)
  - zone5.tar.gz: valid (5 bytes, 112b21db93f16057fa957a58b711bff40ec395c9147d4084a88c9085c6507bf7)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Expanded the comment here and below.

Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

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

Looks good. Someday we will fix the redundant M.2 issues for all consumers.


#[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema, Serialize)]
pub struct ZoneArtifactInventory {
/// The filename.
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these comments written by an LLM? They don't seem very helpful.

I do, on the other hand, think that header docs for each type could be useful. For example, what is a ZoneManifest or MupdateOverride.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually no, I handwrote them 😭 -- I just like all pub fields being documented. I'll try adding some more information like "always ends with .tar.gz".

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, that was quite rude and untactful of me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Expanded the doc comments significantly, think it's much better now -- thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Expanded the doc comments significantly, think it's much better now -- thanks!

Looks great!

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
@sunshowers sunshowers enabled auto-merge (squash) June 23, 2025 20:44
@sunshowers sunshowers merged commit 45cea5d into main Jun 23, 2025
18 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/13n-send-resolver-status-up-via-inventory branch June 23, 2025 21:38
sunshowers added a commit that referenced this pull request Jun 24, 2025
Was working in this area, thought it would be a nice cleanup.

Depends on:

* #8300
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.

4 participants