[SPARK-21715][WebUI] History Server should not respond history page html content multiple times for only one http request#18941
[SPARK-21715][WebUI] History Server should not respond history page html content multiple times for only one http request#18941zhouyejoe wants to merge 0 commit intoapache:masterfrom
Conversation
|
Can one of the admins verify this patch? |
ajbozarth
left a comment
ajbozarth
left a comment
There was a problem hiding this comment.
I'm ok adding these as long as you double check the images meet license requirements and you fix the spacing bug. In the screenshot the column name overlaps with the sorting image, I'm not sure why, but it should be addressed.
|
|
||
| def commonHeaderNodes: Seq[Node] = { | ||
| <meta http-equiv="Content-type" content="text/html; charset=utf-8" /> | ||
| <link rel="shortcut icon" href="about:blank"/> |
There was a problem hiding this comment.
Why exactly is this needed? Your description was vague on the why
There was a problem hiding this comment.
In the past week, I was trying to find out a solution for the overlaps but I didn't make it successfully, as I am not so familiar with CSS and frontend stuffs. I will try to find some professions in our company to solve this issue.
Additionally, this line will disable the downloading for the favicon.icon.
There was a problem hiding this comment.
From my understanding this line is less of a fix and more of a hack. If you have front end dev willing to take a look, have them check this too.
There was a problem hiding this comment.
Definitely I will. Another thing I found that this is not the problem with the standalone spark history server, but also exists in the the live UI running in the driver for a running application.
There was a problem hiding this comment.
The issues you're trying to address are generally harmless errors across the entire UI used by partial imports of 3rd party libraries, which is common in web dev. I'm still a fan of addressing them though.
|
Can you write a proper PR title? It doesn't say what the change is about - nor is it a complete sentence. |
zhouyejoe
changed the title
zhouyejoe
changed the title
|
The PR title does not match what the PR summary says. The title is about one change, the summary is about a different change, and the code seems to handle both. It's all pretty confusing. It seems like Alex's comments weren't yet addressed either. |
4 participants
What changes were proposed in this pull request?
Add the images required by dataTables jQuery plugin that will show up in the history page table header. All the images are downloaded from the official github site for dataTables. Change the path for the images in the dataTables.bootstrap.css to make sure the images can be found. Disable the favicon.ico downloading by changing the header in the html.
How was this patch tested?
Build and unit test passed.
Deployed and check the UI changes. The images are getting correctly downloaded and History Server can handle these requests correctly.