RodrigoDLopez commented on a change in pull request #4230:
URL: https://github.com/apache/cloudstack/pull/4230#discussion_r465668970
##########
File path:
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -814,6 +824,161 @@ public Configuration updateConfiguration(final
UpdateCfgCmd cmd) throws InvalidP
}
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_CONFIGURATION_VALUE_EDIT,
eventDescription = "resetting configuration")
+ public Pair<Configuration, String> resetConfiguration(final ResetCfgCmd
cmd) throws InvalidParameterValueException {
+ final Long userId = CallContext.current().getCallingUserId();
+ final String name = cmd.getCfgName();
+ final Long zoneId = cmd.getZoneId();
+ final Long clusterId = cmd.getClusterId();
+ final Long storagepoolId = cmd.getStoragepoolId();
+ final Long accountId = cmd.getAccountId();
+ final Long domainId = cmd.getDomainId();
+ final Long imageStoreId = cmd.getImageStoreId();
+ Optional optionalValue;
+ final ConfigKey<?> configKey = _configDepot.get(name);
+ if (configKey == null) {
+ s_logger.warn("Probably the component manager where configuration
variable " + name + " is defined needs to implement Configurable interface");
+ throw new InvalidParameterValueException("Config parameter with
name " + name + " doesn't exist");
+ }
+ String defaultValue = configKey.defaultValue();
+ String category = configKey.category();
+ String configScope = configKey.scope().toString();
+
+ String scope = ConfigKey.Scope.Global.toString();
+
+ Long id = null;
+ int paramCountCheck = 0;
+
+ if (zoneId != null) {
+ scope = ConfigKey.Scope.Zone.toString();
+ id = zoneId;
+ paramCountCheck++;
+ }
+ if (clusterId != null) {
+ scope = ConfigKey.Scope.Cluster.toString();
+ id = clusterId;
+ paramCountCheck++;
+ }
+ if (domainId != null) {
+ scope = ConfigKey.Scope.Domain.toString();
+ id = domainId;
+ paramCountCheck++;
+ }
+ if (accountId != null) {
+ scope = ConfigKey.Scope.Account.toString();
+ id = accountId;
+ paramCountCheck++;
+ }
+ if (storagepoolId != null) {
+ scope = ConfigKey.Scope.StoragePool.toString();
+ id = storagepoolId;
+ paramCountCheck++;
+ }
+ if (imageStoreId != null) {
+ scope = ConfigKey.Scope.ImageStore.toString();
+ id = imageStoreId;
+ paramCountCheck++;
+ }
+
+ if (paramCountCheck > 1) {
+ throw new InvalidParameterValueException("cannot handle multiple
IDs, provide only one ID corresponding to the scope");
+ }
+
+ if (scope != null && !scope.equals(ConfigKey.Scope.Global.toString())
&& !configScope.contains(scope)) {
+ throw new InvalidParameterValueException("Invalid scope id
provided for the parameter " + name);
+ }
+
+ String newValue = null;
+ switch (ConfigKey.Scope.valueOf(scope)) {
+ case Zone:
+ final DataCenterVO zone = _zoneDao.findById(id);
Review comment:
And here, you could extract any of this case to an smalll method, add
some documentation and unit test for this news methods.
##########
File path: engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
##########
@@ -103,18 +103,31 @@ public void persist(long clusterId, Map<String, String>
details) {
expunge(sc);
for (Map.Entry<String, String> detail : details.entrySet()) {
+ String name = detail.getKey();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
+ name = "cpuOvercommitRatio";
+ }
+ if (name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ name = "memoryOvercommitRatio";
+ }
String value = detail.getValue();
if ("password".equals(detail.getKey())) {
value = DBEncryptionUtil.encrypt(value);
}
- ClusterDetailsVO vo = new ClusterDetailsVO(clusterId,
detail.getKey(), value);
+ ClusterDetailsVO vo = new ClusterDetailsVO(clusterId, name, value);
persist(vo);
}
txn.commit();
}
@Override
public void persist(long clusterId, String name, String value) {
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
Review comment:
If you extract an method you can reuse it here.
##########
File path: engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java
##########
@@ -103,18 +103,31 @@ public void persist(long clusterId, Map<String, String>
details) {
expunge(sc);
for (Map.Entry<String, String> detail : details.entrySet()) {
+ String name = detail.getKey();
+ if (name.equalsIgnoreCase("cpu.overprovisioning.factor")) {
+ name = "cpuOvercommitRatio";
+ }
+ if (name.equalsIgnoreCase("mem.overprovisioning.factor")) {
+ name = "memoryOvercommitRatio";
+ }
Review comment:
I think you could extract this lines into an new method.
Maybe extract those Strings into constants
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
