Copilot commented on code in PR #134:
URL: https://github.com/apache/cloudstack-go/pull/134#discussion_r3714349427
##########
cloudstack/ManagementService.go:
##########
@@ -755,6 +688,393 @@ type ManagementServersMetric struct {
Version string `json:"version"`
}
+type RemoveManagementServerParams struct {
+ p map[string]interface{}
+}
+
+func (p *RemoveManagementServerParams) 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 *RemoveManagementServerParams) SetId(v string) {
+ if p.p == nil {
+ p.p = make(map[string]interface{})
+ }
+ p.p["id"] = v
+}
+
+func (p *RemoveManagementServerParams) ResetId() {
+ if p.p != nil && p.p["id"] != nil {
+ delete(p.p, "id")
+ }
+}
+
+func (p *RemoveManagementServerParams) 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
RemoveManagementServerParams instance,
+// as then you are sure you have configured all required params
+func (s *ManagementService) NewRemoveManagementServerParams(id string)
*RemoveManagementServerParams {
+ p := &RemoveManagementServerParams{}
+ p.p = make(map[string]interface{})
+ p.p["id"] = id
+ return p
+}
+
+// Removes a Management Server.
+func (s *ManagementService) RemoveManagementServer(p
*RemoveManagementServerParams) (*RemoveManagementServerResponse, error) {
+ resp, err := s.cs.newPostRequest("removeManagementServer",
p.toURLValues())
+ if err != nil {
+ return nil, err
+ }
+
+ var r RemoveManagementServerResponse
+ if err := json.Unmarshal(resp, &r); err != nil {
+ return nil, err
+ }
+
+ return &r, nil
+}
+
+type RemoveManagementServerResponse struct {
+ Displaytext string `json:"displaytext"`
+ JobID string `json:"jobid"`
+ Jobstatus int `json:"jobstatus"`
+ Success bool `json:"success"`
+}
+
+func (r *RemoveManagementServerResponse) 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
+ }
+ }
+
+ type alias RemoveManagementServerResponse
+ return json.Unmarshal(b, (*alias)(r))
+}
Review Comment:
`RemoveManagementServerResponse` doesn’t define an `ostypeid` field, but
`UnmarshalJSON` still rewrites `m["ostypeid"]`. This looks like leftover
copy/paste logic and makes the response handling harder to reason about; please
remove the `ostypeid` conversion block. As an additional improvement, consider
avoiding the repeated `json.Marshal` + `json.Unmarshal` cycles by decoding into
a small intermediate struct (or using a custom “string-or-bool” type for
`success`) to keep the logic targeted and easier to maintain.
--
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]