Copilot commented on code in PR #143:
URL: https://github.com/apache/cloudstack-go/pull/143#discussion_r3718398774
##########
generate/generate.go:
##########
@@ -537,7 +539,9 @@ func (as *allServices) GeneralCode() ([]byte, error) {
pn(" currentTime := time.Now().Unix()")
pn("")
pn(" for {")
- pn(" p := cs.Asyncjob.NewQueryAsyncJobResultParams(jobid)")
+ pn(" p := &QueryAsyncJobResultParams{}")
+ pn(" p.p = make(map[string]interface{})")
+ pn(" p.SetJobID(jobid)")
Review Comment:
The generated GetAsyncJobResult helper is now built by manually
instantiating QueryAsyncJobResultParams and writing to its internal map. This
duplicates the existing constructor logic and couples the generator to the
params struct internals unnecessarily.
##########
cloudstack/ConsoleEndpointService.go:
##########
@@ -99,7 +99,7 @@ func (s *ConsoleEndpointService)
NewCreateConsoleEndpointParams(virtualmachineid
return p
}
-// Create a console endpoint to connect to a VM console
+// Create a console endpoint to connect to a Instance console
Review Comment:
Grammar: the comment should use "an Instance" (not "a Instance").
##########
generate/requiredParams.go:
##########
@@ -70,6 +70,9 @@ var requiredParams = map[string][]string{
"registerTemplate": {
"displaytext",
},
+ "queryAsyncJobResult": {
+ "jobid",
+ },
Review Comment:
listApis.json defines queryAsyncJobResult.jobid as not required (and, since
4.22.1, supports resourceid+resourcetype as an alternative). Marking jobid as
required here forces NewQueryAsyncJobResultParams(jobid) and makes the
resource-based usage harder/less discoverable (users would need to bypass the
constructor and build params manually).
##########
cloudstack/cloudstack.go:
##########
@@ -489,7 +489,9 @@ func (cs *CloudStackClient) GetAsyncJobResult(jobid string,
timeout int64) (json
currentTime := time.Now().Unix()
for {
- p := cs.Asyncjob.NewQueryAsyncJobResultParams(jobid)
+ p := &QueryAsyncJobResultParams{}
+ p.p = make(map[string]interface{})
+ p.SetJobID(jobid)
Review Comment:
GetAsyncJobResult manually constructs QueryAsyncJobResultParams and
initializes its internal map, even though AsyncjobService already provides
NewQueryAsyncJobResultParams(jobid). Using the constructor avoids depending on
the params struct internals and keeps required-param initialization in one
place.
##########
cloudstack/GuestOSService.go:
##########
@@ -2226,3 +2257,342 @@ type GetHypervisorGuestOsNamesResponseGuestoslist
struct {
Osdisplayname string `json:"osdisplayname"`
Osnameforhypervisor string `json:"osnameforhypervisor"`
}
+
+type AddOsCategoryParams struct {
+ p map[string]interface{}
+}
+
+func (p *AddOsCategoryParams) toURLValues() url.Values {
+ u := url.Values{}
+ if p.p == nil {
+ return u
+ }
+ if v, found := p.p["isfeatured"]; found {
+ vv := strconv.FormatBool(v.(bool))
+ u.Set("isfeatured", vv)
+ }
+ if v, found := p.p["name"]; found {
+ u.Set("name", v.(string))
+ }
+ return u
+}
+
+func (p *AddOsCategoryParams) SetIsfeatured(v bool) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ p.p["isfeatured"] = v
+}
+
+func (p *AddOsCategoryParams) ResetIsfeatured() {
+ if p.p != nil && p.p["isfeatured"] != nil {
+ delete(p.p, "isfeatured")
+ }
+}
+
+func (p *AddOsCategoryParams) GetIsfeatured() (bool, bool) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ value, ok := p.p["isfeatured"].(bool)
+ return value, ok
+}
+
+func (p *AddOsCategoryParams) SetName(v string) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ p.p["name"] = v
+}
+
+func (p *AddOsCategoryParams) ResetName() {
+ if p.p != nil && p.p["name"] != nil {
+ delete(p.p, "name")
+ }
+}
+
+func (p *AddOsCategoryParams) GetName() (string, bool) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ value, ok := p.p["name"].(string)
+ return value, ok
+}
+
+// You should always use this function to get a new AddOsCategoryParams
instance,
+// as then you are sure you have configured all required params
+func (s *GuestOSService) NewAddOsCategoryParams(name string)
*AddOsCategoryParams {
+ p := &AddOsCategoryParams{}
+ p.p = make(map[string]interface{})
+ p.p["name"] = name
+ return p
+}
+
+// Adds a new OS category
+func (s *GuestOSService) AddOsCategory(p *AddOsCategoryParams)
(*AddOsCategoryResponse, error) {
+ resp, err := s.cs.newPostRequest("addOsCategory", p.toURLValues())
+ if err != nil {
+ return nil, err
+ }
+
+ var nested struct {
+ Response AddOsCategoryResponse `json:"oscategory"`
+ }
+ if err := json.Unmarshal(resp, &nested); err != nil {
+ return nil, err
+ }
+ r := nested.Response
+
+ return &r, nil
+}
+
+type AddOsCategoryResponse struct {
+ Created string `json:"created"`
+ Icon interface{} `json:"icon"`
+ Id string `json:"id"`
+ Isfeatured bool `json:"isfeatured"`
+ JobID string `json:"jobid"`
+ Jobstatus int `json:"jobstatus"`
+ Name string `json:"name"`
+}
+
+type DeleteOsCategoryParams struct {
+ p map[string]interface{}
+}
+
+func (p *DeleteOsCategoryParams) toURLValues() url.Values {
+ u := url.Values{}
+ if p.p == nil {
+ return u
+ }
+ if v, found := p.p["id"]; found {
+ u.Set("id", v.(string))
+ }
+ return u
+}
+
+func (p *DeleteOsCategoryParams) SetId(v string) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ p.p["id"] = v
+}
+
+func (p *DeleteOsCategoryParams) ResetId() {
+ if p.p != nil && p.p["id"] != nil {
+ delete(p.p, "id")
+ }
+}
+
+func (p *DeleteOsCategoryParams) GetId() (string, bool) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ value, ok := p.p["id"].(string)
+ return value, ok
+}
+
+// You should always use this function to get a new DeleteOsCategoryParams
instance,
+// as then you are sure you have configured all required params
+func (s *GuestOSService) NewDeleteOsCategoryParams(id string)
*DeleteOsCategoryParams {
+ p := &DeleteOsCategoryParams{}
+ p.p = make(map[string]interface{})
+ p.p["id"] = id
+ return p
+}
+
+// Deletes an OS category
+func (s *GuestOSService) DeleteOsCategory(p *DeleteOsCategoryParams)
(*DeleteOsCategoryResponse, error) {
+ resp, err := s.cs.newPostRequest("deleteOsCategory", p.toURLValues())
+ if err != nil {
+ return nil, err
+ }
+
+ var r DeleteOsCategoryResponse
+ if err := json.Unmarshal(resp, &r); err != nil {
+ return nil, err
+ }
+
+ return &r, nil
+}
+
+type DeleteOsCategoryResponse struct {
+ Displaytext string `json:"displaytext"`
+ JobID string `json:"jobid"`
+ Jobstatus int `json:"jobstatus"`
+ Success bool `json:"success"`
+}
+
+func (r *DeleteOsCategoryResponse) UnmarshalJSON(b []byte) error {
+ var m map[string]interface{}
+ err := json.Unmarshal(b, &m)
+ if err != nil {
+ return err
+ }
+
+ if success, ok := m["success"].(string); ok {
+ m["success"] = success == "true"
+ b, err = json.Marshal(m)
+ if err != nil {
+ return err
+ }
+ }
+
+ if ostypeid, ok := m["ostypeid"].(float64); ok {
+ m["ostypeid"] = strconv.Itoa(int(ostypeid))
+ b, err = json.Marshal(m)
+ if err != nil {
+ return err
+ }
+ }
Review Comment:
DeleteOsCategoryResponse.UnmarshalJSON contains a conversion block for
"ostypeid", but DeleteOsCategoryResponse does not have an ostypeid field. This
dead code adds extra marshal/unmarshal work and is likely a copy/paste artifact.
##########
cloudstack/VolumeService.go:
##########
@@ -185,7 +185,7 @@ func (s *VolumeService) NewAttachVolumeParams(id string,
virtualmachineid string
return p
}
-// Attaches a disk volume to a virtual machine.
+// Attaches a disk volume to an Instance.
Review Comment:
Extra whitespace in the comment ("to an") makes it harder to read; it
should be a single space.
##########
cloudstack/ISOService.go:
##########
@@ -151,7 +151,7 @@ func (s *ISOService) NewAttachIsoParams(id string,
virtualmachineid string) *Att
return p
}
-// Attaches an ISO to a virtual machine.
+// Attaches an ISO to an Instance.
Review Comment:
Extra whitespace in the comment ("to an") makes it harder to read; it
should be a single space.
--
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]
