You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/08/15 23:53:21 UTC
[GitHub] [spark] tianhanhu opened a new pull request #29442: Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
tianhanhu opened a new pull request #29442:
URL: https://github.com/apache/spark/pull/29442
### What changes were proposed in this pull request?
Introduced showSessionLink argument to SqlStatsPagedTable class in ThriftServerPage. When this argument is set to true, "Session ID" tooltip will be shown to the user.
### Why are the changes needed?
To show "Session ID" tooltip to users.
### Does this PR introduce _any_ user-facing change?
Yes. Users would be able to see "Session ID"
### How was this patch tested?
Manual Test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #29442: [SPARK-32627][SQL] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-674676317
cc @gengliangwang and @sarutak FYI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #29442: [SPARK-32627][SQL][WEBUI] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #29442:
URL: https://github.com/apache/spark/pull/29442#discussion_r471813284
##########
File path: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerPage.scala
##########
@@ -206,18 +208,34 @@ private[ui] class SqlStatsPagedTable(
override def headers: Seq[Node] = {
val sqlTableHeadersAndTooltips: Seq[(String, Boolean, Option[String])] =
- Seq(
- ("User", true, None),
- ("JobID", true, None),
- ("GroupID", true, None),
- ("Start Time", true, None),
- ("Finish Time", true, Some(THRIFT_SERVER_FINISH_TIME)),
- ("Close Time", true, Some(THRIFT_SERVER_CLOSE_TIME)),
- ("Execution Time", true, Some(THRIFT_SERVER_EXECUTION)),
- ("Duration", true, Some(THRIFT_SERVER_DURATION)),
- ("Statement", true, None),
- ("State", true, None),
- ("Detail", true, None))
+ if (showSessionLink) {
+ Seq(
+ ("User", true, None),
+ ("JobID", true, None),
+ ("GroupID", true, None),
+ ("Session ID", true, None),
+ ("Start Time", true, None),
+ ("Finish Time", true, Some(THRIFT_SERVER_FINISH_TIME)),
+ ("Close Time", true, Some(THRIFT_SERVER_CLOSE_TIME)),
+ ("Execution Time", true, Some(THRIFT_SERVER_EXECUTION)),
+ ("Duration", true, Some(THRIFT_SERVER_DURATION)),
+ ("Statement", true, None),
+ ("State", true, None),
+ ("Detail", true, None))
+ } else {
+ Seq(
+ ("User", true, None),
+ ("JobID", true, None),
+ ("GroupID", true, None),
+ ("Start Time", true, None),
+ ("Finish Time", true, Some(THRIFT_SERVER_FINISH_TIME)),
+ ("Close Time", true, Some(THRIFT_SERVER_CLOSE_TIME)),
+ ("Execution Time", true, Some(THRIFT_SERVER_EXECUTION)),
+ ("Duration", true, Some(THRIFT_SERVER_DURATION)),
+ ("Statement", true, None),
+ ("State", true, None),
+ ("Detail", true, None))
Review comment:
The code in the `if` block and the `else` block are almost same except for `("Session ID", true, None)`.
Could you refactor the code?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29442: Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-674459406
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on pull request #29442: [SPARK-32627][SQL][WEBUI] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-675152279
@tianhanhu Thank you for your contribution.
We have a test suite for the ThriftServer's UI (org.apache.spark.sql.hive.thriftserver.UISeleniumSuite).
So could you add a testcase to the suite?
You can write the test by referencing the other `UISeleniumSuite` like `org.apache.spark.ui.UISeleniumSuite`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #29442: [SPARK-32627][SQL][WEBUI] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #29442:
URL: https://github.com/apache/spark/pull/29442
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29442: Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-674459406
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29442: Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-674459509
Can one of the admins verify this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] gengliangwang commented on pull request #29442: [SPARK-32627][SQL][WEBUI] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
gengliangwang commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-677049610
+1 with adding unit test 👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] commented on pull request #29442: [SPARK-32627][SQL][WEBUI] Add showSessionLink parameter to SqlStatsPagedTable class in ThriftServerPage
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #29442:
URL: https://github.com/apache/spark/pull/29442#issuecomment-735311304
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org