zhuangchong commented on a change in pull request #5200:
URL: 
https://github.com/apache/incubator-dolphinscheduler/pull/5200#discussion_r610999550



##########
File path: 
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java
##########
@@ -420,9 +419,7 @@ public ExecutionStatus getApplicationStatus(String 
applicationId) throws Excepti
 
         String result = Constants.FAILED;
         String applicationUrl = getApplicationUrl(applicationId);
-        if (logger.isDebugEnabled()) {
-            logger.debug("generate yarn application url, applicationUrl={}", 
applicationUrl);
-        }
+        logger.info("applicationUrl={}", applicationUrl);

Review comment:
       The outer loop calls this method, and if the log level is changed to 
INFO here, This information will always be printed(the YARN task runtime)

##########
File path: 
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java
##########
@@ -435,9 +432,7 @@ public ExecutionStatus getApplicationStatus(String 
applicationId) throws Excepti
         } else {
             //may be in job history
             String jobHistoryUrl = getJobHistoryUrl(applicationId);
-            if (logger.isDebugEnabled()) {
-                logger.debug("generate yarn job history application url, 
jobHistoryUrl={}", jobHistoryUrl);
-            }
+            logger.info("jobHistoryUrl={}", jobHistoryUrl);

Review comment:
       The outer loop calls this method, and if the log level is changed to 
INFO here, This information will always be printed(the YARN task runtime)

##########
File path: 
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/HadoopUtils.java
##########
@@ -612,7 +607,9 @@ public static String getAppAddress(String appAddress, 
String rmHa) {
             return null;
         }
 
-        String end = Constants.COLON + split2[1];
+        String activeResourceManagerPort = 
String.valueOf(PropertyUtils.getInt(Constants.HADOOP_RESOURCE_MANAGER_HTTPADDRESS_PORT,
 8088));
+        String rest = split2[1].substring(activeResourceManagerPort.length());
+        String end = Constants.COLON + activeResourceManagerPort + rest;

Review comment:
       1. How do you configure this property 'yarn.application.status.address' 
in the common.properties file?
   
   2. In the common.properties file, the property 
'yarn.application.status.address=http://ds1:8088/ws/v1/cluster/apps/%s' have 
configured in the port, the changes of the code is also increase the port 
handling, This will send a conflict.
   




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


Reply via email to