stmcginnis / gofish

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

Add network protocols support #242

Closed kozl closed 1 year ago

kozl commented 1 year ago

Hi! Thank you very much for this library. I needed support of the NetworkProtocol resource of Manager, so here is my contribution. NetworkProtocolSettings struct is based on this schema

kozl commented 1 year ago

Is there support for updating fields like NetworkProtocolSettings.NTP.NTPServers ? I saw your comment here. Looks like it's not, am I right?

stmcginnis commented 1 year ago

This is looking great!

Yeah, we might need to try out the Update handling with something like this and see if anything needs to be done. It might be able to just pass the array of values, but I haven't had a chance to look too closely and can't recall off the top of my head.

kozl commented 1 year ago

Ok, I'll try to add structures and arrays support in Update, I think today if it won't be very hard. Hope you could release this changes as soon as you can, I need network protocol support in my project. Thank you!

kozl commented 1 year ago

Ok, I'm done. I've put the payload logic in a separate function, write tests on it and add support for updating slices and structs.

kozl commented 1 year ago

Fixed failed tests (global strings.Reader was read twice)

stmcginnis commented 1 year ago

Still a few things to address from the earlier job failures. Will wait to approve the workflow run for that.

Thanks for the updates!

kozl commented 1 year ago

Is there something I need to do, to merge this?

stmcginnis commented 1 year ago

Still a few things to address from the earlier job failures. Will wait to approve the workflow run for that.

Thanks for the updates!

Yep, there were a few failures that haven't been addressed yet. Once those are resolved, this looks good! Running make locally will run the same jobs as the GitHub Actions.

Sorry, looks like there is also a merge conflict now, so you will also need to rebase the latest from the main branch.

kozl commented 1 year ago

Could you point out the issues I need to fix? The only thing I can see here is that «1 workflow awaiting approval». No logs, no errors, looks like Github actions didn't run at all. When running make locally I see lots of unrelated errors like this:

NFO [runner] linters took 10.234178965s with stages: goanalysis_metalinter: 2.118685929s
serviceroot.go:227:14: serviceroot.SetClient undefined (type Service has no field or method SetClient) (typecheck)
    serviceroot.SetClient(c)
                ^
serviceroot.go:233:51: serviceroot.GetClient undefined (type *Service has no field or method GetClient) (typecheck)
    return redfish.ListReferencedChassis(serviceroot.GetClient(), serviceroot.chassis)
                                                     ^
serviceroot.go:238:52: serviceroot.GetClient undefined (type *Service has no field or method GetClient) (typecheck)
    return redfish.ListReferencedManagers(serviceroot.GetClient(), serviceroot.managers)
                                                      ^
serviceroot.go:243:60: serviceroot.GetClient undefined (type *Service has no field or method GetClient) (typecheck)
    return swordfish.ListReferencedStorageSystems(serviceroot.GetClient(), serviceroot.storageSystems)
                                                              ^
serviceroot_test.go:108:12: result.ID undefined (type Service has no field or method ID) (typecheck)
    if result.ID != "ServiceRoot-1" {
              ^
serviceroot_test.go:109:46: result.ID undefined (type Service has no field or method ID) (typecheck)
        t.Errorf("Received invalid ID: %s", result.ID)
                                                   ^
serviceroot_test.go:112:12: result.Name undefined (type Service has no field or method Name) (typecheck)
    if result.Name != "ServiceRootOne" {
              ^
serviceroot_test.go:113:48: result.Name undefined (type Service has no field or method Name) (typecheck)
        t.Errorf("Received invalid name: %s", result.Name)
                                                     ^
serviceroot_test.go:227:12: result.ID undefined (type Service has no field or method ID) (typecheck)
    if result.ID != "ServiceRoot-1" {
              ^
kozl commented 1 year ago

Could we merge this now? Anything else left?

stmcginnis commented 1 year ago

Almost there! image

kozl commented 1 year ago

Fixed nolint comment issues and added exclude-rules part in golang-ci lint (I suppose we don't want to check the length of testing functions)