stmcginnis / gofish

Gofish is a Golang client library for DMTF Redfish and SNIA Swordfish interaction.
BSD 3-Clause "New" or "Revised" License
213 stars 112 forks source link

Fix type of PortLinkStatus constants #319

Closed gnuoy closed 5 months ago

gnuoy commented 5 months ago

Closes: #318

stmcginnis commented 5 months ago

Looking at this now.

Just to note: I edited the description. If you use one of the expected formats, GitHub will automatically link the Issue and the PR so that when the PR merges, it will automatically close the Issue and the two of them will have a reference to each other. Just pointing out for future reference. ;)

stmcginnis commented 5 months ago

Good catch!

There are a few definitions where the names needed to be changed do to the spec using the same name for multiple enums or structs. This appears to be one of the cases where EthernetInterface had a LinkStatus definition:

"LinkStatus": {
            "enum": [
                "LinkUp",
                "NoLink",
                "LinkDown"
            ],
            "enumDescriptions": {
                "LinkDown": "No link is detected on this interface, but the interface is connected.",
                "LinkUp": "The link is available for communication on this interface.",
                "NoLink": "No link or connection is detected on this interface."
            },
            "type": "string"
        }

And the newer Port object also added a LinkStatus:

"LinkStatus": {
            "enum": [
                "LinkUp",
                "Starting",
                "Training",
                "LinkDown",
                "NoLink"
            ],
            "enumDescriptions": {
                "LinkDown": "The link on this interface is down.",
                "LinkUp": "This link on this interface is up.",
                "NoLink": "No physical link detected on this interface.",
                "Starting": "This link on this interface is starting.  A physical link has been established, but the port is not able to transfer data.",
                "Training": "This physical link on this interface is training."
            },
            "type": "string"
        }

And to make it even better, they are almost the same thing, but just different enough that they really should be treated separately. It looks like it would be possible to change the original LinkStatus to just have a few more defined types, but that runs the risk of future updates making incompatible changes. So where this has happened in gofish, the newer (usually) type has been prepended with the associated object name, hence in this particular instance the LinkStatus associated with Port becoming PortLinksStatus in the library.

It looks like when this was done, part of updating for that name change was missed, causing the PortLinksStatus values being set as LinkStatus types.

Thanks for finding this and fixing!