[
https://issues.apache.org/jira/browse/MAPREDUCE-7266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17064813#comment-17064813
]
Siddharth Ahuja edited comment on MAPREDUCE-7266 at 3/23/20, 1:50 PM:
----------------------------------------------------------------------
Based on https://builds.apache.org/job/PreCommit-YARN-Build/25733/console, I
see:
{code}
-1 overall
| Vote | Subsystem | Runtime | Comment
============================================================================
| 0 | reexec | 0m 41s | Docker mode activated.
| | | | Prechecks
| +1 | @author | 0m 0s | The patch does not contain any @author
| | | | tags.
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or
| | | | modified test files.
| | | | trunk Compile Tests
| 0 | mvndep | 0m 36s | Maven dependency ordering for branch
| +1 | mvninstall | 20m 13s | trunk passed
| +1 | compile | 1m 48s | trunk passed
| +1 | checkstyle | 0m 38s | trunk passed
| +1 | mvnsite | 0m 57s | trunk passed
| +1 | shadedclient | 15m 46s | branch has no errors when building and
| | | | testing our client artifacts.
| +1 | findbugs | 1m 8s | trunk passed
| +1 | javadoc | 0m 41s | trunk passed
| | | | Patch Compile Tests
| 0 | mvndep | 0m 11s | Maven dependency ordering for patch
| +1 | mvninstall | 0m 53s | the patch passed
| +1 | compile | 1m 42s | the patch passed
| +1 | javac | 1m 42s | the patch passed
| +1 | checkstyle | 0m 32s |
| | | |
hadoop-mapreduce-project/hadoop-mapreduc
| | | | e-client: The patch generated 0 new +
| | | | 21 unchanged - 1 fixed = 21 total (was
| | | | 22)
| +1 | mvnsite | 0m 48s | the patch passed
| +1 | whitespace | 0m 0s | The patch has no whitespace issues.
| +1 | shadedclient | 14m 13s | patch has no errors when building and
| | | | testing our client artifacts.
| +1 | findbugs | 1m 21s | the patch passed
| +1 | javadoc | 0m 35s | the patch passed
| | | | Other Tests
| +1 | unit | 3m 45s | hadoop-mapreduce-client-hs in the
| | | | patch passed.
| +1 | unit | 121m 55s | hadoop-mapreduce-client-jobclient in
| | | | the patch passed.
| -1 | asflicense | 0m 36s | The patch generated 1 ASF License
| | | | warnings.
| | | 188m 39s |
{code}
So the issue seems to be as described here:
https://builds.apache.org/job/PreCommit-YARN-Build/25733/artifact/out/patch-asflicense-problems.txt
{code}
Lines that start with ????? in the ASF License report indicate files that do
not have an Apache license header:
!?????
/testptch/hadoop/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/jobTokenPassword
{code}
However, my patch doesn't touch this non-java file at all, as such, based on
discussions with [~snemeth], we can ignore this one.
Therefore, I believe that once the code review is confirmed, considering the
patch build reports no other issues, we should be good with this one.
was (Author: sahuja):
Based on based on
https://builds.apache.org/job/PreCommit-YARN-Build/25733/console, I see:
{code}
-1 overall
| Vote | Subsystem | Runtime | Comment
============================================================================
| 0 | reexec | 0m 41s | Docker mode activated.
| | | | Prechecks
| +1 | @author | 0m 0s | The patch does not contain any @author
| | | | tags.
| +1 | test4tests | 0m 0s | The patch appears to include 1 new or
| | | | modified test files.
| | | | trunk Compile Tests
| 0 | mvndep | 0m 36s | Maven dependency ordering for branch
| +1 | mvninstall | 20m 13s | trunk passed
| +1 | compile | 1m 48s | trunk passed
| +1 | checkstyle | 0m 38s | trunk passed
| +1 | mvnsite | 0m 57s | trunk passed
| +1 | shadedclient | 15m 46s | branch has no errors when building and
| | | | testing our client artifacts.
| +1 | findbugs | 1m 8s | trunk passed
| +1 | javadoc | 0m 41s | trunk passed
| | | | Patch Compile Tests
| 0 | mvndep | 0m 11s | Maven dependency ordering for patch
| +1 | mvninstall | 0m 53s | the patch passed
| +1 | compile | 1m 42s | the patch passed
| +1 | javac | 1m 42s | the patch passed
| +1 | checkstyle | 0m 32s |
| | | |
hadoop-mapreduce-project/hadoop-mapreduc
| | | | e-client: The patch generated 0 new +
| | | | 21 unchanged - 1 fixed = 21 total (was
| | | | 22)
| +1 | mvnsite | 0m 48s | the patch passed
| +1 | whitespace | 0m 0s | The patch has no whitespace issues.
| +1 | shadedclient | 14m 13s | patch has no errors when building and
| | | | testing our client artifacts.
| +1 | findbugs | 1m 21s | the patch passed
| +1 | javadoc | 0m 35s | the patch passed
| | | | Other Tests
| +1 | unit | 3m 45s | hadoop-mapreduce-client-hs in the
| | | | patch passed.
| +1 | unit | 121m 55s | hadoop-mapreduce-client-jobclient in
| | | | the patch passed.
| -1 | asflicense | 0m 36s | The patch generated 1 ASF License
| | | | warnings.
| | | 188m 39s |
{code}
So the issue seems to be as described here:
https://builds.apache.org/job/PreCommit-YARN-Build/25733/artifact/out/patch-asflicense-problems.txt
{code}
Lines that start with ????? in the ASF License report indicate files that do
not have an Apache license header:
!?????
/testptch/hadoop/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/jobTokenPassword
{code}
However, my patch doesn't touch this non-java file at all, as such, based on
discussions with [~snemeth], we can ignore this one.
Therefore, I believe that once the code review is confirmed, considering the
patch build reports no other issues, we should be good with this one.
> historyContext doesn't need to be a class attribute inside JobHistoryServer
> ---------------------------------------------------------------------------
>
> Key: MAPREDUCE-7266
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-7266
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: jobhistoryserver
> Reporter: Siddharth Ahuja
> Priority: Minor
> Attachments: YARN-10075.001.patch
>
>
> "historyContext" class attribute at
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L67
> is assigned a cast of another class attribute - "jobHistoryService" -
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L131,
> however it does not need to be stored separately because it is only ever
> used once in the clas, and that too as an argument while instantiating the
> HistoryClientService class at
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L155.
> Therefore, we could just delete the lines at
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L67
> and
> https://github.com/apache/hadoop/blob/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/JobHistoryServer.java#L131
> completely and instantiate the HistoryClientService as follows:
> {code}
> @VisibleForTesting
> protected HistoryClientService createHistoryClientService() {
> return new HistoryClientService((HistoryContext)jobHistoryService,
> this.jhsDTSecretManager);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
