add request string logging to be able to get more information #155
Conversation
banyan/resource_accesstier.go
Outdated
| return nil, fmt.Errorf("invalid ID (%s), expected name:<name of access tier to import>", parts) | ||
| } | ||
| name := parts[1] | ||
| accessTierClient := i.(*client.Holder) |
There was a problem hiding this comment.
would it be good to check if the type assertion succeeded?
There was a problem hiding this comment.
yes, sure shall add.
client/restclient/restclient.go
Outdated
| uerr := json.Unmarshal(responseData, &errResp) | ||
| if uerr == nil { | ||
| myErr := json.Unmarshal(responseData, &errResp) | ||
| if myErr == nil { | ||
| err = fmt.Errorf("recieved error code %d: %s \n response: \n %s", response.StatusCode, requestStr, responseData) |
There was a problem hiding this comment.
misspelling of "received"
There was a problem hiding this comment.
should this function also return an err here when myErr is non-nil?
There was a problem hiding this comment.
i had same thought when i looked into, but looks like idea of the HandleResponse function was to print corresponding errors based on http error code, so when there are parsing errors it is left to the caller to handle. in this above case we need to set err to myErr.
There was a problem hiding this comment.
it's not returning or handling myErr at all currently.
There was a problem hiding this comment.
yes i saw it, made corresponding change.
client/restclient/restclient.go
Outdated
| @@ -235,8 +244,8 @@ func HandleResponse(response *http.Response, requestStr string) (responseData [] | |||
| if err != nil { | |||
There was a problem hiding this comment.
This condition is never true
There was a problem hiding this comment.
yes so removed
There was a problem hiding this comment.
| return j.Data, nil | ||
| for _, accessTier := range j.Data.AccessTiers { | ||
| if accessTier.Name == name { | ||
| spec = accessTier |
There was a problem hiding this comment.
should you add a break statement here?
There was a problem hiding this comment.
yes correct shall add
Add request logging to get more insight when there is a failure