Copilot commented on code in PR #13884:
URL: https://github.com/apache/cloudstack/pull/13884#discussion_r3917368696


##########
engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java:
##########
@@ -398,7 +396,7 @@ public void copyAsync(DataObject srcdata, DataObject 
destData, AsyncCompletionCa
                 (srcdata.getType() == DataObjectType.SNAPSHOT && 
destData.getType() == DataObjectType.SNAPSHOT) ||
                 (srcdata.getType() == DataObjectType.VOLUME && 
destData.getType() == DataObjectType.VOLUME)) {
 
-            int nMaxExecutionMinutes = 
NumbersUtil.parseInt(configDao.getValue(Config.SecStorageCmdExecutionTimeMax.key()),
 30);
+            int nMaxExecutionMinutes = 
AgentManager.SecStorageCmdExecutionTimeMax.value();

Review Comment:
   `AgentManager` is referenced here but the file no longer imports it (it 
removed the old `Config` import). This will not compile unless `AgentManager` 
is imported or the reference is fully qualified.



##########
usage/src/main/java/com/cloud/usage/UsageManagerImpl.java:
##########
@@ -229,16 +230,16 @@ public boolean configure(String name, Map<String, Object> 
params) throws Configu
             throw new CloudRuntimeException("Unhandled configuration 
exception", e);
         }
 
-        String execTime = configs.get("usage.stats.job.exec.time");
-        String aggregationRange = 
configs.get("usage.stats.job.aggregation.range");
-        String execTimeZone = configs.get("usage.execution.timezone");
-        String aggregationTimeZone = configs.get("usage.aggregation.timezone");
-        String sanityCheckInterval = 
configs.get("usage.sanity.check.interval");
+        String execTime = UsageService.UsageStatsJobExecTime.value();
+        Integer aggregationRange = 
UsageService.UsageStatsJobAggregationRange.value();
+        String execTimeZone = UsageService.UsageExecutionTimezone.value();
+        String aggregationTimeZone = 
UsageService.UsageAggregationTimezone.value();
+        Integer sanityCheckInterval = 
UsageService.UsageSanityCheckInterval.value();

Review Comment:
   `configure()` still builds/merges the `configs` map (including `params` 
overrides) but the usage-job settings are now read via `ConfigKey.value()`, 
which ignores the merged `configs`/`params`. This changes behavior vs. the 
previous implementation and makes overrides passed via `params` ineffective. 
Consider reading from `configs` first (falling back to the `ConfigKey` 
default/global value) so `mergeConfigs(configs, params)` remains meaningful for 
these settings.



-- 
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]

Reply via email to