[ https://issues.apache.org/jira/browse/YARN-6475?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16003682#comment-16003682 ]
Daniel Templeton commented on YARN-6475: ---------------------------------------- Thanks for working on the patch, [~soumabrata]. Looks generally good. A few comments: * In {{buildContainerRuntimeContext()}}, {{runAsUser}} is only used once, so you may as well inline it like you did with the rest of the attribute values. * In {{deriveContainerWorkDir()}}, the {{StringBuilder}} is unnecessary. The compiler will turn + concatenation into a {{StringBuilder}} under the covers. If you think + concatenation would be easier to read, feel free to use it. Or indeed, this change may not be necessary. * In {{prepareContainer()}}, your code: {code} exec.prepareContainer(new ContainerPrepareContext.Builder() .setContainer(container) .setLocalizedResources(localResources) .setUser(container.getUser()) .setContainerLocalDirs(containerLocalDirs) .setCommands(container.getLaunchContext() .getCommands()).build());{code} might be better formatted as: {code} exec.prepareContainer(new ContainerPrepareContext.Builder() .setContainer(container) .setLocalizedResources(localResources) .setUser(container.getUser()) .setContainerLocalDirs(containerLocalDirs) .setCommands(container.getLaunchContext().getCommands()) .build());{code} * In {{StatusUpdaterRunnable}}, we don't need an empty line after the class declaration. * In {{StatusUpdaterRunnable}}, it looks like you have several places with 8-space indentation where 4-space should do. * In {{StatusUpdaterRunnable.run()}}, you may as well replace {{if (containersToSignal.size() != 0)}} with {{if (!containersToSignal.isEmpty())}} * In {{StatusUpdaterRunnable.run()}}, you should also fix the {{catch (Throwable)}}. Either make it a multi-catch instead or just catch {{Exception}}. Also, please post a patch from the PR to this JIRA so that the Jenkins pre-commit has something to chew on. > Fix some long function checkstyle issues > ---------------------------------------- > > Key: YARN-6475 > URL: https://issues.apache.org/jira/browse/YARN-6475 > Project: Hadoop YARN > Issue Type: Bug > Reporter: Miklos Szegedi > Assignee: Soumabrata Chakraborty > Priority: Trivial > Labels: newbie > > I am talking about these two: > {code} > ./hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java:441: > @Override:3: Method length is 176 lines (max allowed is 150). [MethodLength] > ./hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java:159: > @Override:3: Method length is 158 lines (max allowed is 150). [MethodLength] > {code} -- This message was sent by Atlassian JIRA (v6.3.15#6346) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org