Copilot commented on code in PR #217:
URL:
https://github.com/apache/cloudstack-cloudmonkey/pull/217#discussion_r3727872521
##########
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 {
+ fmt.Println("Error: empty API list received, sync failed")
+ return nil
+ }
Review Comment:
This nil check avoids the original panic, but it still (1) writes the error
to stdout, (2) hard-codes "sync" in the message even though UpdateCache is also
called from LoadCache, and (3) returns early after having already cleared the
global caches (apiCache/apiVerbMap), which can wipe a previously loaded cache
on transient server issues. Also, if response["api"] is non-nil but not a
[]interface{}, the type assertion will still panic. Consider validating with a
safe type assertion, reporting to stderr, and only resetting caches after
validation; return 0 (or another sentinel) so callers like sync don’t print
"<nil>".
--
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]
