Copilot commented on code in PR #13884:
URL: https://github.com/apache/cloudstack/pull/13884#discussion_r4324813063
##########
engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java:
##########
@@ -138,6 +138,23 @@ public interface NetworkOrchestrationService {
ConfigKey<Integer> VmNetworkThrottlingRate = new
ConfigKey<Integer>("Network", Integer.class, "vm.network.throttling.rate",
"200",
"Default data transfer rate in megabits per second allowed in User
vm's default network.", true, ConfigKey.Scope.Zone);
+ ConfigKey<String> NetworkLBHaproxyStatsVisbility = new
ConfigKey<>("Network", String.class,
+ "network.loadbalancer.haproxy.stats.visibility", "global",
+ "Load Balancer(haproxy) stats visibility, the value can be one of
the following six parameters :
global,guest-network,link-local,disabled,all,default",
+ true, ConfigKey.Kind.Select,
"global,guest-network,link-local,disabled,all,default");
+
+ ConfigKey<String> NetworkLBHaproxyStatsUri = new ConfigKey<>("Network",
String.class,
+ "network.loadbalancer.haproxy.stats.uri", "/admin?stats",
+ "Load Balancer(haproxy) uri.", true);
+
+ ConfigKey<String> NetworkLBHaproxyStatsAuth = new ConfigKey<>("Secure",
String.class,
+ "network.loadbalancer.haproxy.stats.auth", "admin1:AdMiN123",
+ "Load Balancer(haproxy) authentication string in the format
username:password", true);
+
+ ConfigKey<String> NetworkLBHaproxyStatsPort = new ConfigKey<>("Network",
String.class,
+ "network.loadbalancer.haproxy.stats.port", "8081",
+ "Load Balancer(haproxy) stats port number.", true);
Review Comment:
The HAProxy stats port is modeled as a `String` but is consumed numerically
elsewhere (e.g., parsed via `Integer.parseInt(...)`). Defining it as
`ConfigKey<Integer>` would provide type-safety and earlier validation (and
remove repeated parsing). If you must keep it as a string for backward
compatibility, consider at least adding validation at the boundary where it’s
used to ensure the value is numeric and within a valid port range.
##########
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ObjectStoreDaoImpl.java:
##########
@@ -26,21 +26,21 @@
import com.cloud.utils.db.TransactionStatus;
import org.apache.cloudstack.api.response.ObjectStoreResponse;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.framework.config.ConfigKey;
import org.springframework.stereotype.Component;
-import javax.inject.Inject;
import javax.naming.ConfigurationException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@Component
public class ObjectStoreDaoImpl extends GenericDaoBase<ObjectStoreVO, Long>
implements ObjectStoreDao {
+ // Registered/owned by ManagementServer; declared here too so this
lower-layer module can read it without depending on the server module.
+ private static final ConfigKey<Integer> DetailBatchQuerySize = new
ConfigKey<>("Advanced", Integer.class, "detail.batch.query.size", "2000",
+ "Default entity detail batch query size for listing", true);
Review Comment:
This PR introduces multiple local duplicated `ConfigKey` declarations for
the same underlying setting (`detail.batch.query.size`) across different
modules/DAOs. That duplication increases the risk of drift
(defaults/descriptions/category diverging). Consider centralizing this
`ConfigKey` into a shared API/components module accessible from these layers,
or introducing a single shared constants class for cross-module config keys.
--
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]
