sv2000 commented on a change in pull request #3207:
URL: https://github.com/apache/incubator-gobblin/pull/3207#discussion_r562157414
##########
File path:
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnHelixUtils.java
##########
@@ -54,6 +55,7 @@
*
* @author Yinan Li
*/
+@Slf4j
Review comment:
Is this annotation needed? Looks like we are initializing a Logger
already.
##########
File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java
##########
@@ -516,7 +518,13 @@ protected ContainerLaunchContext
newContainerLaunchContext(Container container,
if
(this.config.hasPath(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY)) {
addRemoteAppFiles(this.config.getString(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY),
resourceMap);
}
-
+ if
(this.config.hasPath(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY)) {
+
addRemoteAppFiles(this.config.getString(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY),
resourceMap);
+ }
+ if
(this.config.hasPath(GobblinYarnConfigurationKeys.APP_MASTER_ZIPS_REMOTE_KEY)) {
Review comment:
Should we have a separate config called CONTAINER_ZIPS_REMOTE_KEY?
##########
File path:
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinYarnAppLauncher.java
##########
@@ -791,6 +798,20 @@ private void addAppRemoteFiles(String hdfsFileList,
Map<String, LocalResource> r
}
}
+ private void addAppRemoteZips(String hdfsFileList, Map<String,
LocalResource> resourceMap)
Review comment:
Can this method be moved to YarnHelixUtils to avoid code duplication in
YarnService?
##########
File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java
##########
@@ -516,7 +518,13 @@ protected ContainerLaunchContext
newContainerLaunchContext(Container container,
if
(this.config.hasPath(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY)) {
addRemoteAppFiles(this.config.getString(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY),
resourceMap);
}
-
+ if
(this.config.hasPath(GobblinYarnConfigurationKeys.CONTAINER_FILES_REMOTE_KEY)) {
Review comment:
Lines 521-523 are duplicate of lines 518-520 and can be removed.
----------------------------------------------------------------
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]
