New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[KYUUBI #6358] Fix the incorrect construction of yarnConf #6386
base: master
Are you sure you want to change the base?
Conversation
Looking for a pro to fix this issue.馃榿 |
@@ -56,6 +56,7 @@ trait ApplicationOperation { | |||
* @note For implementations, please suppress exceptions and always return KillResponse | |||
*/ | |||
def killApplicationByTag( | |||
sessionConf: Option[KyuubiConf], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sessionConf: Option[KyuubiConf], | |
def killApplicationByTag( | |
appMgrInfo: ApplicationManagerInfo, | |
tag: String, | |
proxyUser: Option[String] = None, | |
sessionConf: Option[KyuubiConf] = None): KillResponse |
case e: Exception => | ||
(false, s"Failed to terminate application with $tag, due to ${e.getMessage}") | ||
proxyUser: Option[String] = None): KillResponse = | ||
withNewYarnClient(proxyUser, sessionConf) { yarnClient => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that newYarnClient
is not always needed. Can we use it only when needed? Like, the hadoop_conf_dir/yarn_conf_dir
engine evn exists in sessionConf
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your help, you're right, I'll make the changes.
Utils.doAs(user) { () => | ||
var yarnClient: YarnClient = null | ||
try { | ||
val yarnConf = KyuubiHadoopUtils.newYarnConfiguration(sessionConf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this use kyuubi.engineEnv.HADOOP_CONF_DIR/YARN_CONF_DIR
in sessionConf to initialize yarnConf
?
馃攳 Description
Issue References 馃敆
#6385
Describe Your Solution 馃敡
This PR is just for better explaining this issue and for testing. The fix is quite crude, and there are still many issues not addressed.
Types of changes 馃敄
Test Plan 馃И
Behavior Without This Pull Request 鈿帮笍
Behavior With This Pull Request 馃帀
Related Unit Tests
Checklist 馃摑
Be nice. Be informative.