Skip to content
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

[SAPRK-20785][WEB-UI][SQL] Spark should provide jump links and add (count) in the SQL web ui. #19346

Closed
wants to merge 2 commits into from

Conversation

guoxiaolongzte
Copy link

What changes were proposed in this pull request?

propose:

it provide links that jump to Running Queries,Completed Queries and Failed Queries.
it add (count) about Running Queries,Completed Queries and Failed Queries.
This is a small optimization in in the SQL web ui.

fix before:

1

fix after:
2

How was this patch tested?

manual tests

Please review http://spark.apache.org/contributing.html before opening a pull request.

@guoxiaolongzte
Copy link
Author

@HyukjinKwon @jerryshao @ajbozarth
In the new PR I fix the indentation problem.
Thanks.

The latest verification results are as follows:
3

@jerryshao
Copy link
Contributor

ok to test.

@jerryshao
Copy link
Contributor

Please fix the title.

@guoxiaolongzte guoxiaolongzte changed the title Spark should provide jump links and add (count) in the SQL web ui. [SAPRK-20785][WEB-UI][SQL] Spark should provide jump links and add (count) in the SQL web ui. Sep 26, 2017
@SparkQA
Copy link

SparkQA commented Sep 26, 2017

Test build #82171 has finished for PR 19346 at commit 305689f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jerryshao
Copy link
Contributor

GLTM. @gatorsmile , would you please take a look at this PR, is it good for you?

@guoxiaolongzte
Copy link
Author

@gatorsmile Help to review the code.

}
{
if (listener.getCompletedExecutions.nonEmpty) {
<li id="completed-summary">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think completed-summary is for ui test on jobs and stages pages. We may no need to use it here if no test for it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I do not quite understand what you mean. Here is just show statistics. Let the SQL UI and other UI interface style consistent.

Job ui:
1

Copy link
Member

@viirya viirya Sep 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find the style for completed-summary. Do we have defined style for it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said the style is to show how much.

1.master ui:
1

2.worker ui:
2

3.job ui:
4

4.stage ui:
3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant id=completed-summary looks like is basically used for ui test against jobs and stages pages. I can't find we actually defined a style for it. If you don't add a test for it in this execution page, we may not need to add it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not talking this kind of style. I meant any css style defined for id=completed-summary...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what you mean. I will delete it. Thank you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply said, I think we can just write <li> instead of <li id="completed-summary"> at this line.

@viirya
Copy link
Member

viirya commented Sep 27, 2017

LGTM

@SparkQA
Copy link

SparkQA commented Sep 27, 2017

Test build #82222 has finished for PR 19346 at commit 279e0d2.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member

viirya commented Sep 27, 2017

retest this please.

@SparkQA
Copy link

SparkQA commented Sep 27, 2017

Test build #82224 has finished for PR 19346 at commit 279e0d2.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 27, 2017

Test build #82225 has finished for PR 19346 at commit 279e0d2.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@guoxiaolongzte
Copy link
Author

retest this please.

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Sep 27, 2017

Test build #82234 has finished for PR 19346 at commit 279e0d2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jerryshao
Copy link
Contributor

OK, let me merge to master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants