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

Bump dependencies, progenitor to dfc6e0f2, oxide.json to omicron:33bbad37 #915

Merged
merged 11 commits into from
Nov 15, 2024

Conversation

oxide-reflector-bot[bot]
Copy link
Contributor

@oxide-reflector-bot oxide-reflector-bot bot commented Nov 13, 2024

Lock file updated

Bump progenitor from 789c4b66 to dfc6e0f2
Changes: oxidecomputer/progenitor@789c4b6...dfc6e0f

Generated code against nexus.json 33bbad37

@oxide-reflector-bot oxide-reflector-bot bot changed the title Bump dependencies, progenitor to dfc6e0f2, oxide.json to omicron:2ef7a772 Bump dependencies, progenitor to dfc6e0f2, oxide.json to omicron:3cbc9539 Nov 14, 2024
@oxide-reflector-bot oxide-reflector-bot bot changed the title Bump dependencies, progenitor to dfc6e0f2, oxide.json to omicron:3cbc9539 Bump dependencies, progenitor to dfc6e0f2, oxide.json to omicron:33bbad37 Nov 14, 2024
@david-crespo
Copy link
Contributor

Hey @Nieuwejaar, could you check my fixes for the optional FEC link thing in 5c6e928? Wasn't sure if it should be None or Some(LinkFec::None).

CliCommand::TimeseriesQuery => Some("experimental timeseries query"),
CliCommand::TimeseriesSchemaList => Some("experimental timeseries schema list"),
CliCommand::SystemTimeseriesQuery => Some("experimental timeseries query"),
CliCommand::SystemTimeseriesSchemaList => Some("experimental timeseries schema list"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Leaning against adding system here — it's already experimental and I'm not sure we're going to bother adding the project-scoped one from oxidecomputer/omicron#6873 when it lands.

@Nieuwejaar
Copy link

Hey @Nieuwejaar, could you check my fixes for the optional FEC link thing in 5c6e928? Wasn't sure if it should be None or Some(LinkFec::None).

Either one would be valid, but using one of each would give broader test coverage.

@david-crespo
Copy link
Contributor

That occurred to me, will do it.

@david-crespo
Copy link
Contributor

had to update a snapshot

test test_port_config ... FAILED

failures:

---- test_port_config stdout ----
@@ -14,12 +14,12 @@
 Destination  Nexthop     Vlan  Preference
 1.2.3.0/24   1.2.3.4/32  1701  10
 
 switch1/qsfp0
 =============
-Autoneg  Fec   Speed
-false    None  Speed100G
+Autoneg  Fec         Speed
+false    Some(None)  Speed100G
 
 Address          Lot            VLAN
 169.254.20.2/30  initial-infra  None
 169.254.40.2/30  initial-infra  Some(400)
 

thread 'test_port_config' panicked at /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/expectorate-1.1.0/src/feature_predicates.rs:41:17:
assertion failed: string doesn't match the contents of file: "tests/data/test_switch_port_settings_show.stdout" see diffset above
                set EXPECTORATE=overwrite if these changes are intentional

Autoneg Fec Speed
false None Speed100G
Autoneg Fec Speed
false Some(None) Speed100G
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like odd output from this command. I'll look into it

Comment on lines 1 to 7
switch0/qsfp0
=============
Autoneg Fec Speed
false None Speed100G
Autoneg Fec Speed
false <auto> speed100_g

Address Lot VLAN
169.254.10.2/30 initial-infra None
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rcgoodfellow I changed this to have what I thought was more reasonable output (i.e. not using Debug). If you don't like it, please let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks fine to me

@ahl ahl merged commit af7502b into main Nov 15, 2024
16 checks passed
@ahl ahl deleted the integration branch November 15, 2024 23:44
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