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
move dpd-client dependency out of omicron common #5738
Conversation
/// Opaque object representing link state. The contents of this object are not | ||
/// yet stable. | ||
#[derive(Clone, Debug, Deserialize, Serialize)] | ||
pub struct SwitchLinkState { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bulk of this PR is moving this type to nexus/types/src/external_api/shared.rs
This breaks downstreams that import omicron_common as the way dpd-client gets built is specific to the omicron build machinery.
e5bf1d1
to
da728b2
Compare
oximeter imports this and oximeter is imported by downstreams
@@ -27,7 +27,6 @@ http.workspace = true | |||
ipnetwork.workspace = true | |||
macaddr.workspace = true | |||
mg-admin-client.workspace = true | |||
dpd-client.workspace = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes downstream breakage, as compilation of this crate requires omicron-specific build machinery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah... I feel like CI should catch this, maybe by building a stub project with omicron-common as its only dependency. Could you file an issue for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
@@ -27,7 +27,6 @@ http.workspace = true | |||
ipnetwork.workspace = true | |||
macaddr.workspace = true | |||
mg-admin-client.workspace = true | |||
dpd-client.workspace = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah... I feel like CI should catch this, maybe by building a stub project with omicron-common as its only dependency. Could you file an issue for this?
This breaks downstreams that import omicron_common as the way dpd-client gets built is specific to the omicron build machinery.
For example, this happens when trying to bump omicron in maghemite.