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

Swtich NetworkAdapter fields from common.LinksCollection to common.Link #329

Merged
merged 1 commit into from
May 6, 2024

Conversation

smiller248
Copy link
Contributor

This changes a number of properties of the NetworkAdapter resource type (NetworkPorts, Ports, Processors, etc.) to be parsed as common.Link instead of common.LinksCollection. Meaning that each of these properties will now be parsed a single link to a collection resource rather than as an array of links.

(For NetworkDeviceFunctions and NetworkPorts, the flip from common.LinksCollection to common.Link reverses the change that occurred in 646d2ab. The other properties were newly added to gofish.)

This is necessary forNewtworkAdapter to parse correctly. Please correct me if I'm somehow confused here, but the spec for the latest NetworkAdapter schema (https://redfish.dmtf.org/schemas/v1/NetworkAdapter.v1_10_0.json) states that all of these fields are single links, not arrays. (I think there may have been some confusion due toControllerLinks having child properties with the same names that are in fact arrays, not single links, e.g. there is ControllerLinks.NetworkPorts which is an array).

Copy link
Owner

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

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

Thanks! There were definitely a few spots where this error was made when adding a lot of new objects in the last big update. Thanks for catching that.

It would be great to extend the unit test JSON to include these to make sure everything parses correctly, but I've had to fix up a few like this elsewhere so I'm pretty confident this is good.

@stmcginnis stmcginnis merged commit 70e8a4b into stmcginnis:main May 6, 2024
2 checks passed
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