bgeng777 commented on a change in pull request #18739:
URL: https://github.com/apache/flink/pull/18739#discussion_r816582211



##########
File path: 
flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -1829,4 +1809,41 @@ public static void logDetachedClusterInformation(
                 yarnApplicationId,
                 yarnApplicationId);
     }
+
+    @VisibleForTesting
+    Map<String, String> generateApplicationMasterEnv(
+            final YarnApplicationFileUploader fileUploader,
+            final String classPathStr,
+            final String localFlinkJarStr,
+            final String appIdStr)
+            throws IOException {
+        final Map<String, String> env = new HashMap<>();
+        // set YARN classpath
+        env.put(ENV_FLINK_CLASSPATH, classPathStr);

Review comment:
       Changed it back.
   At first, I hope to extract some common config setting that will be used 
both JM and TM container and adjust the order. Later, I find the gain of such 
refactor is small compared to its risk. When rolling back, the order is not 
considered(my bad). Though I think the order should be not crucial, I agree the 
change had better be as small as possible.




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