Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sled-agent] address EarlyNetworkConfig issues from 20231130 dogfood mupdate #4636

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Dec 6, 2023

  • Log deserialization as both v1 and v0 failing.
  • If an error occurs with deserialization as both v1 and v0, then preferentially return the error produced by v1 rather than v0.
  • Add tests for bootstore blobs from earlier versions. If more fields are added in the future, this test should catch any issues that occur.

Created using spr 1.3.5
Created using spr 1.3.5
/// Test that previous and current versions of `EarlyNetworkConfig` blobs
/// deserialize correctly.
#[test]
fn early_network_blobs_deserialize() {
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW: for little parsers like this, I really like using nom.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah definitely if it's a bit more complex than this. In this case I figured it was just splitting by comma.

Copy link
Contributor

@rcgoodfellow rcgoodfellow left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@sunshowers sunshowers merged commit e0882ae into main Dec 7, 2023
19 of 20 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/sled-agent-address-earlynetworkconfig-issues-from-20231130-dogfood-mupdate branch December 7, 2023 01:26
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