-
Notifications
You must be signed in to change notification settings - Fork 990
Fix kyuubi sessions to return 404 for non-existent sessions #7319
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
Changes from 2 commits
a4d81e9
79f6406
14c03d9
aade31c
0b0d0e7
e2e3fd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,11 +52,15 @@ private[v1] class SessionsResource extends ApiRequestContext with Logging { | |
| content = Array(new Content( | ||
| mediaType = MediaType.APPLICATION_JSON, | ||
| array = new ArraySchema(schema = new Schema(implementation = classOf[SessionData])))), | ||
| description = "get the list of all live sessions") | ||
| description = "get the list of live sessions for the current user") | ||
| @GET | ||
| def sessions(): Seq[SessionData] = { | ||
| sessionManager.allSessions() | ||
| .map(session => ApiUtils.sessionData(session.asInstanceOf[KyuubiSession])).toSeq | ||
| val userName = fe.getSessionUser(Map.empty[String, String]) | ||
| sessionManager | ||
| .allSessions() | ||
| .filter(session => session.user == userName) | ||
| .map(session => ApiUtils.sessionData(session.asInstanceOf[KyuubiSession])) | ||
| .toSeq | ||
| } | ||
|
|
||
| @ApiResponse( | ||
|
|
@@ -157,8 +161,15 @@ private[v1] class SessionsResource extends ApiRequestContext with Logging { | |
| @Path("{sessionHandle}") | ||
| def closeSession(@PathParam("sessionHandle") sessionHandleStr: String): Response = { | ||
| info(s"Received request of closing $sessionHandleStr") | ||
| fe.be.closeSession(sessionHandleStr) | ||
| Response.ok().build() | ||
| try { | ||
| fe.be.closeSession(sessionHandleStr) | ||
| Response.ok().build() | ||
| } catch { | ||
| case e: org.apache.kyuubi.KyuubiSQLException | ||
| if e.getMessage != null && e.getMessage.startsWith("Invalid ") => | ||
| throw new NotFoundException( | ||
|
||
| ApiUtils.logAndRefineErrorMsg(e.getMessage, e)) | ||
| } | ||
| } | ||
|
|
||
| @ApiResponse( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,6 +40,12 @@ import org.apache.kyuubi.session.SessionType | |||||||||
|
|
||||||||||
| class SessionsResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { | ||||||||||
|
|
||||||||||
| override protected lazy val conf: KyuubiConf = { | ||||||||||
| val c = KyuubiConf() | ||||||||||
| c.set(KyuubiConf.SERVER_SECRET_REDACTION_PATTERN, "(?i)password".r) | ||||||||||
| c | ||||||||||
| } | ||||||||||
|
|
||||||||||
| override protected def beforeEach(): Unit = { | ||||||||||
| super.beforeEach() | ||||||||||
| eventually(timeout(10.seconds), interval(200.milliseconds)) { | ||||||||||
|
|
@@ -389,4 +395,27 @@ class SessionsResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper { | |||||||||
| assert(operations.size == 1) | ||||||||||
| assert(sessionHandle.toString.equals(operations.head.getSessionId)) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| test("get /sessions returns redacted spark confs") { | ||||||||||
|
||||||||||
| test("get /sessions returns redacted spark confs") { | |
| test("get /sessions returns redacted spark confs") { | |
| // Configure redaction so that sensitive configs like spark.password are masked | |
| conf.set(KyuubiConf.SERVER_SECRET_REDACTION_PATTERN, "(?i)password".r) |
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.
how about only fix the 404 issue in this PR?
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.
sure, I will only include 404 fix and revert others