vishesh92 commented on code in PR #8151: URL: https://github.com/apache/cloudstack/pull/8151#discussion_r1378652762
########## api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListTrafficTypesCmd.java: ########## @@ -27,14 +27,13 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.PhysicalNetworkResponse; -import org.apache.cloudstack.api.response.ProviderResponse; import org.apache.cloudstack.api.response.TrafficTypeResponse; import com.cloud.network.PhysicalNetworkTrafficType; import com.cloud.user.Account; import com.cloud.utils.Pair; -@APICommand(name = "listTrafficTypes", description = "Lists traffic types of a given physical network.", responseObject = ProviderResponse.class, since = "3.0.0", +@APICommand(name = "listTrafficTypes", description = "Lists traffic types of a given physical network.", responseObject = TrafficTypeResponse.class, since = "3.0.0", Review Comment: @rohityadavcloud @kishankavala # tl;dr; We will need to make this change. Because cloudstack-go is currently based on output of `listApis` response and having a wrong response here will generate an incorrect client and terraform provider won't be able to use the keys we add in this PR. Only keys which are present in both `ProviderResponse` & `TrafficTypeResponse` are accessible as of now using the cloudstack-go client. So, I don't think backward compatibility will be an issue here. ## Details In cloudstack-go client's TrafficType struct, it has some keys which are not part of `listTrafficType` response and missing keys which should be in the response. Ref: https://github.com/apache/cloudstack-go/blob/c9c595770d7dcc2c87090227ef15c94ceee27b2b/cloudstack/UsageService.go#L1092-L1107 This is the difference in output of cmk & cloudstack-go: cmk ``` { "count": 2, "traffictype": [ { "id": "04q1bbe1-0e1e-4a1a-94e3-btcd257z44b9", "kvmnetworklabel": "cloudbr0", "physicalnetworkid": "47191b54-510c-475e-va6f-d935e22a9744", "traffictype": "Management", "vmwarenetworklabel": "vSwitch0,,vmwaresvs", "xennetworklabel": "Network0" } ] } ``` cloudstack-go ``` cloudstack.TrafficType{ Canenableindividualservice: false, Destinationphysicalnetworkid: "", Id: "04q1bbe1-0e1e-4a1a-94e3-btcd257z44b9", JobID: "", Jobstatus: 0, Name: "", Physicalnetworkid: "47191b54-510c-475e-va6f-d935e22a9744", Servicelist: []string(nil), State: "" } ``` P.S. - This is one of my current env and not built on this PR. As you can see there are some keys (`traffictype`, `kvmnetworklabel`, etc.) which are present in response but we can't access them using cloudstack-go client because it's not part of the struct. code to listTrafficType using cloudstack-go ```go func main() { cs := cloudstack.NewAsyncClient( "http://localhost:8080/client/api";, "access-key", "secret-key", false, ) p := cs.Usage.NewListTrafficTypesParams("47191b54-510c-475e-va6f-d935e22a9744"); r, err := cs.Usage.ListTrafficTypes(p) if err != nil { log.Fatalf("%+v", err) } fmt.Printf("%#v\n", *r.TrafficTypes[0]) } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
