Extract all yarn related methods to azkaban-common as shared util library #3145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tsangz2013
commented
Sep 6, 2022
| final String logFilePath = jobProps.getString(CommonJobProperties.JOB_LOG_FILE); | ||
| logger.info("Log file path is: " + logFilePath); | ||
|
|
||
| HadoopJobUtils.proxyUserKillAllSpawnedHadoopJobs(logFilePath, jobProps, tokenFile, logger); |
There was a problem hiding this comment.
Not good: duplicated info in the 2 parameters
tsangz2013
commented
Sep 6, 2022
| @@ -0,0 +1,71 @@ | |||
| package azkaban.utils; | |||
There was a problem hiding this comment.
Basically moved from old HadoopJobUtils, but follow the idea of yarnClient can be passed in
tsangz2013
force-pushed
the
zhzeng/extract-yarn-methods-to-utils
branch
from
74f379a
to
9716e64
Compare
tsangz2013
commented
Sep 6, 2022
Comment on lines
60
to
85
| public static YarnClient createYarnClient(Props props) { | ||
| final YarnConfiguration yarnConf = new YarnConfiguration(); | ||
| final YarnClient yarnClient = YarnClient.createYarnClient(); | ||
| if (props.containsKey(YARN_CONF_DIRECTORY_PROPERTY)) { | ||
| yarnConf.addResource( | ||
| new Path(props.get(YARN_CONF_DIRECTORY_PROPERTY) + "/" + YARN_CONF_FILENAME)); | ||
| } | ||
| yarnClient.init(yarnConf); | ||
| yarnClient.start(); | ||
| return yarnClient; | ||
| } |
There was a problem hiding this comment.
Separate yarnclient creation for later reuse
Codecov Report
@@ Coverage Diff @@
## master #3145 +/- ##
============================================
+ Coverage 41.73% 41.87% +0.14%
- Complexity 4712 4740 +28
============================================
Files 600 606 +6
Lines 40397 40564 +167
Branches 4703 4709 +6
============================================
+ Hits 16860 16988 +128
- Misses 22134 22172 +38
- Partials 1403 1404 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
NancyXie2022
reviewed
Sep 7, 2022
az-hadoop-jobtype-plugin/src/main/java/azkaban/jobtype/HadoopJobUtils.java
Show resolved
Hide resolved
tsangz2013
force-pushed
the
zhzeng/extract-yarn-methods-to-utils
branch
from
9716e64
to
a35ff0e
Compare
|
looks good to me now, ship it. |
NancyXie2022
previously approved these changes
Sep 8, 2022
tsangz2013
force-pushed
the
zhzeng/extract-yarn-methods-to-utils
branch
from
ca8481a
to
a35ff0e
Compare
djaiswal83
reviewed
Sep 8, 2022
tsangz2013
force-pushed
the
zhzeng/extract-yarn-methods-to-utils
branch
from
a35ff0e
to
0a62250
Compare
djaiswal83
requested changes
Sep 8, 2022
tsangz2013
added a commit
to tsangz2013/azkaban
that referenced
this pull request
Sep 15, 2022
…util library (azkaban#3145)" This reverts commit a31c30d.
tsangz2013
added a commit
to tsangz2013/azkaban
that referenced
this pull request
Sep 24, 2022
set HADOOP_TOKEN_FILE_LOCATION to token file add logging around tokens remove the token-file deletion code Extract all yarn related methods to azkaban-common as shared util library (azkaban#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file
tsangz2013
added a commit
that referenced
this pull request
Sep 29, 2022
…il library (#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file
NancyXie2022
pushed a commit
that referenced
this pull request
May 26, 2023
…rary (#3145) * Extract all yarn related methods to azkaban-common as shared util library * extract duplicate code to a static method * amend header for test file RB=3636861 G=azkabandev-reviewers R=anath1,ypadron,angoel,adsharma,jakhani,areznik,gsalia,apruthi,sarumuga,prkhande,abtiwari,djaiswal,sshardoo A=bijiang
NancyXie2022
pushed a commit
that referenced
this pull request
May 26, 2023
…util library (#3145)" (#3154) This reverts commit a31c30d. Reason: this change doesn't work with containerization, will fix the issue and raise a new PR for this. 3637010 RB=3637010 G=azkabandev-reviewers R=ypadron,jakhani,prkhande,sarumuga,angoel,adsharma,abtiwari,gsalia,djaiswal,apruthi,anath1,sshardoo,areznik A=bijiang
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue to solve: A part of the fixing / upgrade of the yarn application killing logic: make it easier to add and reuse "call yarn cluster for application ids" method later on
Changes made: Some simple refactor of the code, extract all the yarn-client related methods to an azkaban-common util s class
Testing done: