csquire commented on a change in pull request #2894: api: don't throttle api
discovery for listApis command
URL: https://github.com/apache/cloudstack/pull/2894#discussion_r224445187
##########
File path:
plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java
##########
@@ -223,7 +223,9 @@ private ApiDiscoveryResponse getCmdRequestMap(Class<?>
cmdClass, APICommand apiC
for (APIChecker apiChecker : _apiAccessCheckers) {
try {
- apiChecker.checkAccess(user, name);
+ if
(!"ApiRateLimitServiceImpl".equals(apiChecker.getName())) {
Review comment:
@rafaelweingartner Maybe I'm missing something with your suggestion?
`ListApisCmd` is available to the discovery service on the classpath (it's not
currently passed into the listApis() method), but I don't see how that helps
choose which APIChecker to ignore unless the cmd class gets passed into
APIChecker.checkAccess(), which would then introduce an api discovery module
dependency on the rate limit plugin module.
It looks like there used to be a separate APILimitChecker interface used for
this, but it appears to have been abandoned years ago. I also don't like hard
coding names, but the only alternative I saw was a much bigger change that
would add risk.
Again, let me know if I misunderstood your suggestion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
