DaanHoogland commented on code in PR #6348:
URL: https://github.com/apache/cloudstack/pull/6348#discussion_r944287338


##########
agent/src/main/java/com/cloud/agent/AgentShell.java:
##########
@@ -265,75 +266,80 @@ protected boolean parseCommand(final String[] args) 
throws ConfigurationExceptio
             }
         }
 
-        if (port == null) {
-            port = getProperty(null, "port");
-        }
-
-        _port = NumberUtils.toInt(port, 8250);
+        setHost(host);
 
-        _proxyPort = NumberUtils.toInt(getProperty(null, 
"consoleproxy.httpListenPort"), 443);
+        _guid = getGuid(guid);
+        _port = getPortOrWorkers(port, AgentProperties.PORT);
+        _workers = getWorkers(workers);
+        _zone = getZoneOrPod(zone, AgentProperties.ZONE);
+        _pod = getZoneOrPod(pod, AgentProperties.POD);
 
-        if (workers == null) {
-            workers = getProperty(null, "workers");
-        }
+        _proxyPort = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CONSOLEPROXY_HTTPLISTENPORT);
+        _pingRetries = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.PING_RETRIES);
+        preferredHostCheckInterval = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_LB_CHECK_INTERVAL);
 
-        _workers = NumberUtils.toInt(workers, 5);
-        if (_workers <= 0) {
-            _workers = 5;
-        }
+        return true;
+    }
 
+    protected void setHost(String host) throws ConfigurationException {
         if (host == null) {
-            host = getProperty(null, "host");
+            host = 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST);
         }
 
-        if (host == null) {
-            host = "localhost";
+        if (isValueStartingAndEndingWithAtSign(host)) {
+            throw new ConfigurationException(String.format("Host [%s] is not 
configured correctly.", host));
         }
 
         setHosts(host);
+    }
 
-        if (zone != null)
-            _zone = zone;
-        else
-            _zone = getProperty(null, "zone");
-        if (_zone == null || (_zone.startsWith("@") && _zone.endsWith("@"))) {
-            _zone = "default";
+    protected boolean isValueStartingAndEndingWithAtSign(String value) {
+        return value.startsWith("@") && value.endsWith("@");
+    }
+
+    protected String getGuid(String guid) throws ConfigurationException {
+        guid = StringUtils.defaultString(guid, 
AgentPropertiesFileHandler.getPropertyValue(AgentProperties.GUID));
+        if (guid != null) {
+            return guid;
         }
 
-        if (pod != null)
-            _pod = pod;
-        else
-            _pod = getProperty(null, "pod");
-        if (_pod == null || (_pod.startsWith("@") && _pod.endsWith("@"))) {
-            _pod = "default";
+        if 
(!AgentPropertiesFileHandler.getPropertyValue(AgentProperties.DEVELOPER)) {

Review Comment:
   ```suggestion
           if 
(Boolean.False.equals(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.DEVELOPER)))
 {
   ```
   note that this is not marked as a bug by sonar because you may have guarded 
against that. It is a code smell because people might change implementation and 
not look here, so an NPE may arise during maintenance.



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