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

[KYUUBI #5035] Spark engine session page display session end time and duration #5038

Closed
wants to merge 3 commits into from

Conversation

lsm1
Copy link
Contributor

@lsm1 lsm1 commented Jul 10, 2023

Why are the changes needed?

close #5035

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate
    image

  • Run test locally before make a pull request

@bowenliang123
Copy link
Contributor

Any screenshots for comparing the changes?

@lsm1
Copy link
Contributor Author

lsm1 commented Jul 11, 2023

Any screenshots for comparing the changes?

ok,like this
image

Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

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

lGTM overall.

@cxzl25 cxzl25 changed the title [KYUUBI 5035][FEATURE] Spark engine session page display session end time and duration [KYUUBI #5035] Spark engine session page display session end time and duration Jul 11, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2023

Codecov Report

Merging #5038 (c68fd65) into master (e36d5d3) will not change coverage.
The diff coverage is 0.00%.

❗ Current head c68fd65 differs from pull request most recent head 9ee06f1. Consider uploading reports for the commit 9ee06f1 to get more accurate results

@@          Coverage Diff           @@
##           master   #5038   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         563     563           
  Lines       31176   31188   +12     
  Branches     4070    4072    +2     
======================================
- Misses      31176   31188   +12     
Impacted Files Coverage Δ
.../scala/org/apache/spark/ui/EngineSessionPage.scala 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

""
} else {
s"""
|Session end at ${formatDate(sessionStat.endTime)},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|Session end at ${formatDate(sessionStat.endTime)},
| ended at ${formatDate(sessionStat.endTime)},

@bowenliang123
Copy link
Contributor

How about changed to separated lines for connection details and the statistics.

        <h4>
          User {sessionStat.username},
          IP {sessionStat.ip},
          Server {sessionStat.serverIp},
        </h4> ++
        <h4>
          Session created at {formatDate(sessionStat.startTime)},
          if (sessionStat.endTime > 0) {
          s"""
             | ended at ${formatDate(sessionStat.endTime)},
             | after ${formatDuration(sessionStat.duration)}.
             |""".stripMargin
        }
          Total run {sessionStat.totalOperations} SQL
        </h4> ++

@bowenliang123
Copy link
Contributor

Would you like to update the screenshots in the description? And I think this PR is good to go. @lsm1

@lsm1
Copy link
Contributor Author

lsm1 commented Jul 12, 2023

Would you like to update the screenshots in the description? And I think this PR is good to go. @lsm1

ok,done

@bowenliang123 bowenliang123 added this to the v1.8.0 milestone Jul 12, 2023
@bowenliang123
Copy link
Contributor

Thanks, merged to master.

link3280 pushed a commit to link3280/kyuubi that referenced this pull request Jul 23, 2023
…me and duration

### _Why are the changes needed?_
close apache#5035

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] Add screenshots for manual tests if appropriate
![image](https://github.com/apache/kyuubi/assets/18713676/502b489f-6cbd-4510-a89c-b7816b3e15bf)
- [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request

Closes apache#5038 from lsm1/features/kyuubi_5035.

Closes apache#5035

9ee06f1 [senmiaoliu] fix style
c68fd65 [senmiaoliu] fix style
5d3d869 [senmiaoliu] show session end time

Authored-by: senmiaoliu <senmiaoliu@trip.com>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Spark engine session page display session end time and duration
5 participants