borisstoyanov commented on code in PR #217:
URL:
https://github.com/apache/cloudstack-cloudmonkey/pull/217#discussion_r3813288472
##########
config/cache.go:
##########
@@ -112,6 +112,10 @@ func (c *Config) UpdateCache(response
map[string]interface{}) interface{} {
apiVerbMap = nil
count := response["count"]
+ if response["api"] == nil {
Review Comment:
This nil check stops the original panic, but a few things are still off:
1. The error goes to stdout, not stderr.
2. The message hard-codes "sync", but `UpdateCache` is also called from
`LoadCache`.
3. `apiCache`/`apiVerbMap` are already cleared before the early return, so a
transient bad response wipes a cache that was previously fine.
4. If `response["api"]` is non-nil but not a `[]interface{}`, the type
assertion still panics.
A safe type assertion placed *before* the reset covers all four. Returning
`0` instead of `nil` also keeps `sync` from printing `Discovered APIs`.
Suggested top of the function (note the two reset lines move down):
```go
func (c *Config) UpdateCache(response map[string]interface{}) interface{} {
apiList, valid := response["api"].([]interface{})
if !valid || len(apiList) == 0 {
fmt.Fprintln(os.Stderr, "Error: no APIs found in the discovery
response, keeping the existing API cache. Please run 'sync'.")
return 0
}
apiCache = make(map[string]*API)
apiVerbMap = nil
count := response["count"]
for _, node := range apiList {
```
`os` is already imported in this file, so no import change is needed.
Tested locally with a cache file of `{"count":0}`,
`{"count":1,"api":"boom"}` and `{"count":0,"api":[]}` — all three print the
error to stderr instead of panicking, and `go vet ./config/` is clean.
--
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]
