-
Notifications
You must be signed in to change notification settings - Fork 919
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[KYUUBI #5966] Error occurs on clicking on the Spark UI Kyuubi Query Engine tab #5967
Conversation
val revision: String = props.getProperty("revision", unknown) | ||
val revisionTime: String = props.getProperty("revision_time", unknown) | ||
val branch: String = Option(props.getProperty("branch", unknown)) | ||
.filterNot(_.isEmpty).getOrElse(unknown) |
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.
let extract a method and guard all properties
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.
@pan3793 good idea,i have updated, please check.
…ring index out of range: 7
3bcd851
to
213493a
Compare
private def getProperty(props: Properties, key: String, defaultValue: String): String = { | ||
Option(props.getProperty(key, defaultValue)).filterNot(_.isEmpty).getOrElse(defaultValue) | ||
} |
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.
we don't need to make it generic, move it to BuildInfo
body and simply write as
private def getProperty(props: Properties, key: String, defaultValue: String): String = { | |
Option(props.getProperty(key, defaultValue)).filterNot(_.isEmpty).getOrElse(defaultValue) | |
} | |
private def getProperty(key: String, defaultValue: String = unknown): String = { | |
Option(props.getProperty(key, defaultValue)).filterNot(_.isEmpty).getOrElse(defaultValue) | |
} |
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.
I agree, thank you.
…ring index out of range: 7
…Engine tab # 🔍 Description ## Issue References 🔗 This pull request fixes #5966 Fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 ## Describe Your Solution 🔧 When the variables branch, revision, revisionTime, repoUrl are empty strings (""),assign them the default value "\<unknown\>". ## Types of changes :bookmark: - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request :coffin: #### Behavior With This Pull Request :tada: #### Related Unit Tests --- # Checklist 📝 - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #5967 from Emor-nj/KyuubiQueryEngineTab. Closes #5966 0cee947 [Cheng Pan] Update kyuubi-common/src/main/scala/org/apache/kyuubi/package.scala 5d8c030 [Cheng Pan] Update kyuubi-common/src/main/scala/org/apache/kyuubi/package.scala 733a9af [Emor-nj] fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 213493a [Emor-nj] fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 Lead-authored-by: Cheng Pan <[email protected]> Co-authored-by: Emor-nj <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit 57d531d) Signed-off-by: Cheng Pan <[email protected]>
Thanks, merged to master/1.8 |
…Query Engine tab # 🔍 Description ## Issue References 🔗 This pull request fixes apache#5966 Fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 ## Describe Your Solution 🔧 When the variables branch, revision, revisionTime, repoUrl are empty strings (""),assign them the default value "\<unknown\>". ## Types of changes :bookmark: - [ ] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 #### Behavior Without This Pull Request :coffin: #### Behavior With This Pull Request :tada: #### Related Unit Tests --- # Checklist 📝 - [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#5967 from Emor-nj/KyuubiQueryEngineTab. Closes apache#5966 0cee947 [Cheng Pan] Update kyuubi-common/src/main/scala/org/apache/kyuubi/package.scala 5d8c030 [Cheng Pan] Update kyuubi-common/src/main/scala/org/apache/kyuubi/package.scala 733a9af [Emor-nj] fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 213493a [Emor-nj] fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7 Lead-authored-by: Cheng Pan <[email protected]> Co-authored-by: Emor-nj <[email protected]> Signed-off-by: Cheng Pan <[email protected]>
🔍 Description
Issue References 🔗
This pull request fixes #5966
Fix Kyuubi Query Engine tab error:StringIndexOutOfBoundsException: String index out of range: 7
Describe Your Solution 🔧
When the variables branch, revision, revisionTime, repoUrl are empty strings (""),assign them the default value "<unknown>".
Types of changes 🔖
Test Plan 🧪
Behavior Without This Pull Request ⚰️
Behavior With This Pull Request 🎉
Related Unit Tests
Checklist 📝
Be nice. Be informative.