-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SPARK-17843][WEB UI] Indicate event logs pending for processing on h… #15991
Conversation
@tgravescs - Looking to get this change early with 2.0.x. This merges cleanly - are you ok to let this into branch-2.0? |
Test build #69061 has finished for PR 15991 at commit
|
|
||
{ | ||
if (lastUpdatedTime > 0) { | ||
<p>Last updated: <span id="last-updated">{lastUpdatedTime}</span></p> |
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.
Is this just going to display a Long to the user? That's not going to be very informative. What about a human time where <= 0 is mapped to "never"?
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.
the javascript on the web page translates the long to a human readable date-time value - in the viewer's (i.e. browser's) local timezone.
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 see —thanks. That's all I was worried about.
@tgravescs - are you ok to merge this backport to branch-2.0 ? |
The reason I hadn't pulled it back is its listed as a minor improvement and generally we only pull defects back into the maintenance releases. |
@tgravescs would like to have this in 2.0 along with the other improvement that got accepted for backport (spark-18010). so would you consider allowing this into 2.0 as well? |
since its just in the history server and pretty isolated seems ok. +1 |
What changes were proposed in this pull request?
Backport PR #15410 to branch-2.0
How was this patch tested?
Existing unit tests. Screenshots for UI changes provided in PR #15410.