julien-vaz commented on code in PR #10892:
URL: https://github.com/apache/cloudstack/pull/10892#discussion_r2166813519
##########
framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java:
##########
@@ -340,12 +349,41 @@ protected List<QuotaUsageVO>
createQuotaUsagesAccordingToQuotaTariffs(AccountVO
}
protected boolean shouldCalculateUsageRecord(AccountVO accountVO, UsageVO
usageRecord) {
- if
(Boolean.FALSE.equals(QuotaConfig.QuotaAccountEnabled.valueIn(accountVO.getAccountId())))
{
+ boolean calculateUsageRecord = findConfigurationValue(accountVO,
QuotaConfig.QuotaAccountEnabled);
+ if (!calculateUsageRecord && usageRecord != null) {
logger.debug("Considering usage record [{}] as calculated and
skipping it because account [{}] has the quota plugin disabled.",
usageRecord.toString(usageAggregationTimeZone),
accountVO.reflectionToString());
return false;
}
- return true;
+ return calculateUsageRecord;
+ }
+
+ @Override
+ public boolean findConfigurationValue(AccountVO accountVO,
ConfigKey<Boolean> key) {
+ logger.trace("Searching configuration [{}] of account [{}] in its
settings.", key.key(), accountVO);
+ AccountDetailVO accountDetail =
accountDetailsDao.findDetail(accountVO.getAccountId(), key.key());
+ if (accountDetail != null) {
+ boolean result = Boolean.parseBoolean(accountDetail.getValue());
+ logger.trace("Using value [{}] found in account [{}] settings to
configuration [{}].", result, accountVO, key.key());
+ return result;
+ }
+
+ if
(Boolean.parseBoolean(_configDao.getValue("enable.account.settings.for.domain")))
{
+ logger.trace("Searching for configuration [{}] of account [{}] in
its domain [{}] settings.", key.key(), accountVO, accountVO.getDomainId());
+ DomainDetailVO domainDetail =
domainDetailsDao.findDetail(accountVO.getDomainId(), key.key());
+ if (domainDetail != null) {
+ boolean result = Boolean.parseBoolean(domainDetail.getValue());
+ logger.trace("Using value [{}] found in domain [{}] settings
to configuration [{}].", result, accountVO.getDomainId(), key.key());
+ return result;
+ }
+ }
+ boolean result =
Boolean.parseBoolean(getConfigValueOrDefaultValue(key));
+ logger.trace("Using default value [{}] to configuration [{}].",
result, key.key());
+ return result;
+ }
Review Comment:
Hm... I guess it's not necessary. My point is that the other parts of the
`findConfigurationValue()` are never reached because the first `valueInScope()`
(needed to retrieve the value in `Account` scope) always return something,
whether the value in the scope used as argument or the value in parent or
global scope.
So, once we call `valueInScope()` to retrieve the value in `Account` scope,
if it doesn't find it, it will search in `Domain` (if we add it to the
`ConfigKey`'s `scopes` in question, of course) by calling
`valueInGlobalOrAvailableParentScope()`.
That been said, we could simplify the `findConfigurationValue()` method by
just calling `valueInScope()`. We would need to add the `Domain` scope to both
configurations' `scopes` and adjust the logs to inform which `scope` will be
used. Just calling `valueInGlobalOrAvailableParentScope()` won't work because
it won't be able to return the value in `Account` scope.
What do you think?
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
