rohityadavcloud commented on PR #8985: URL: https://github.com/apache/cloudstack/pull/8985#issuecomment-2080360286
Thanks @weizhouapache @JoaoJandre I've addressed the feedback and comments. Joao's remarks are valid, however, my comments at the time were largely around metrics vs non-metrics APIs - how they should be used treated. At the time I couldn't co-relate what made the API slow only to later find out through community and customer issues who are facing degraded performance. Generally speaking, we shouldn't break API compatibility which can break potential integrations (as also on this PR Wei has objected). I feel in this case whatever we do we break what was introduced by #5984 in 4.17. The root cause that I could determine is, we accumulate VM stats for each VM in list API calls which isn't a good idea. That said, I think we need a stop-gap until a better solution emerges on how we should process and return stats are returned by the list API (I think we shouldn't reduce/accumulate stats during a list API call). The list VM API response method is used not only for listVMs API but also for several other VM related lifecycle APIs (such as migrate, stop/start etc) which all return the same vm response class/object. So, I've tried to have a different approach with the PR, and introduce a new global setting that can allow users who want backward compatibility but largely benefit users who don't want it (as default). I've updated the description to reflect this. -- 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]
