You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by "weizhouapache (via GitHub)" <gi...@apache.org> on 2026/08/05 08:46:29 UTC
[PR] Fix listing virtual machines with vnfnics [cloudstack-go]
weizhouapache opened a new pull request, #161:
URL: https://github.com/apache/cloudstack-go/pull/161
This fixes #159
--
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: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listing virtual machines with vnfnics [cloudstack-go]
Posted by "abh1sar (via GitHub)" <gi...@apache.org>.
abh1sar merged PR #161:
URL: https://github.com/apache/cloudstack-go/pull/161
--
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: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix listing virtual machines with vnfnics [cloudstack-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #161:
URL: https://github.com/apache/cloudstack-go/pull/161#discussion_r3719309794
##########
generate/generate.go:
##########
@@ -2009,6 +2009,20 @@ func (s *service) generateResponseType(a *API) {
pn("")
return
}
+ if a.Name == "listVnfAppliances" {
+ // The API docs do not describe the shape of the "vnfnics" field, so this
+ // type is hand maintained to mirror org.apache.cloudstack.api.response.VnfNicResponse.
+ pn("type VnfNic struct {")
+ pn(" Deviceid int64 `json:\"deviceid\"`")
+ pn(" Description string `json:\"description\"`")
+ pn(" Management bool `json:\"management\"`")
+ pn(" Name string `json:\"name\"`")
+ pn(" Networkid string `json:\"networkid\"`")
+ pn(" Networkname string `json:\"networkname\"`")
+ pn(" Required bool `json:\"required\"`")
+ pn("}")
+ pn("")
+ }
Review Comment:
The generator only emits the `VnfNic` type when `a.Name == \"listVnfAppliances\"`, but `mapType` now maps any `vnfnics` field to `[]*VnfNic`. If an API spec/version includes `vnfnics` but does not include `listVnfAppliances`, the generated code will reference `VnfNic` without defining it, causing a compile error. Consider emitting `VnfNic` unconditionally in a shared/common types section, or tracking whether `vnfnics` was encountered and emitting the type once whenever it’s needed (independent of a specific API name).
##########
generate/generate.go:
##########
@@ -2403,6 +2417,9 @@ func mapType(aName string, pName string, pType string) string {
if pName == "scaledownpolicies" || pName == "scaleuppolicies" {
return "[]*AutoScalePolicy"
}
+ if pName == "vnfnics" {
+ return "[]*VnfNic"
+ }
Review Comment:
The generator only emits the `VnfNic` type when `a.Name == \"listVnfAppliances\"`, but `mapType` now maps any `vnfnics` field to `[]*VnfNic`. If an API spec/version includes `vnfnics` but does not include `listVnfAppliances`, the generated code will reference `VnfNic` without defining it, causing a compile error. Consider emitting `VnfNic` unconditionally in a shared/common types section, or tracking whether `vnfnics` was encountered and emitting the type once whenever it’s needed (independent of a specific API name).
##########
cloudstack/VirtualNetworkFunctionsService.go:
##########
@@ -3009,6 +3009,16 @@ func (s *VirtualNetworkFunctionsService) ListVnfAppliances(p *ListVnfAppliancesP
return &r, nil
}
+type VnfNic struct {
+ Deviceid int64 `json:"deviceid"`
+ Description string `json:"description"`
+ Management bool `json:"management"`
+ Name string `json:"name"`
+ Networkid string `json:"networkid"`
+ Networkname string `json:"networkname"`
+ Required bool `json:"required"`
+}
Review Comment:
`VnfNic` is used broadly (multiple services now reference it), but it’s defined inside `VirtualNetworkFunctionsService.go`, which makes the type harder to discover and creates implicit coupling between otherwise unrelated services. Consider placing `VnfNic` in a shared/package-level types file (e.g., `cloudstack/types.go`) that’s clearly intended for cross-service DTOs, and updating the generator to emit it there.
##########
cloudstack/VirtualMachineService.go:
##########
@@ -4800,7 +4800,7 @@ type VirtualMachine struct {
Videoram int64 `json:"videoram"`
Vmtype string `json:"vmtype"`
Vnfdetails map[string]string `json:"vnfdetails"`
- Vnfnics []string `json:"vnfnics"`
+ Vnfnics []*VnfNic `json:"vnfnics"`
Review Comment:
Changing `vnfnics` from `[]string` to `[]*VnfNic` across many exported response/resource structs is a breaking public API change for downstream consumers. If backward compatibility is required, consider introducing a dedicated `Vnfnics` type with custom JSON unmarshalling that can accept both legacy (`[]string`) and structured (array of objects) shapes, or providing an alternate accessor while keeping the existing field shape.
--
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: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org