[
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: [email protected]
For additional commands, e-mail: [email protected]
