You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/10/11 09:29:13 UTC
[GitHub] [incubator-kyuubi] lightning-L opened a new pull request, #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with Ad…
lightning-L opened a new pull request, #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609
…minResource
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _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
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#issuecomment-1275517551
https://github.com/apache/incubator-kyuubi/actions/runs/3227353094/jobs/5290613894#step:5:1546
```
- delete engine *** FAILED ***
List("serviceUri=localhost:40647;version=1.7.0-SNAPSHOT;refId=fed46f40-0259-42a9-aed1-bebcd1792b10;sequence=0000000000") had size 1 instead of expected size 0 (AdminResourceSuite.scala:87)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995258494
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
we shall UPPER the shareLevel specified by customer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992228802
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
Review Comment:
after shorten the variable name,
```
s"${fe.getConf.get(HA_NAMESPACE)}_${KYUUBI_VERSION}_${engineShareLevel}_$normalizedEngineType"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995260626
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
let's refactor it with below.
```
val clonedConf = fe.getConf.clone
Option(engineType).foreach(clonedConf.set(ENGINE_TYPE, _))
Option(subdomain).filter(_.nonEmpty).foreach(_ => clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, Option(subdomain))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL, _))
val normalizedEngineType = clonedConf.get(ENGINE_TYPE)
val engineSubdomain = clonedConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default")
val engineShareLevel = clonedConf.get(ENGINE_SHARE_LEVEL)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992091460
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("version") version: String,
+ @QueryParam("namespace") namespace: String): Response = {
Review Comment:
I think `version` and `namespace` are not needed.
cc @pan3793
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995260626
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
let's refactor it with below.
```
val clonedConf = fe.getConf.clone
Option(engineType).foreach(clonedConf.set(ENGINE_TYPE, _))
Option(subdomain).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, Option(_))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL, _))
val normalizedEngineType = clonedConf.get(ENGINE_TYPE)
val engineSubdomain = clonedConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default")
val engineShareLevel = clonedConf.get(ENGINE_SHARE_LEVEL)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995260626
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
let's refactor it with below.
```
val clonedConf = fe.getConf.clone
Option(engineType).foreach(clonedConf.set(ENGINE_TYPE, _))
clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, Option(subdomain).filter(_.nonEmpty))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL, _))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992095827
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
Review Comment:
this is not needed. we shall get username by the authentication
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] lightning-L commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992349781
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
Review Comment:
I am using
```
val engineSubdomain = Option(subdomain).filter(_.nonEmpty).orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN)).getOrElse("default")
```
There will be error for:
```
val engineSubdomain = Option(subdomain).filter(_.nonEmpty).orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
```
<img width="732" alt="image" src="https://user-images.githubusercontent.com/21362040/195108260-d06d0af7-11b5-45cf-9b24-b32ae947f927.png">
Type mismatch:
Required: Option[NotInferred]
Found: String
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995260626
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
let's refactor it with below.
```
val clonedConf = fe.getConf.clone
Option(engineType).foreach(clonedConf.set(ENGINE_TYPE, _))
Option(subdomain).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, Option(subdomain))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL, _))
val normalizedEngineType = clonedConf.get(ENGINE_TYPE)
val engineSubdomain = clonedConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default")
val engineShareLevel = clonedConf.get(ENGINE_SHARE_LEVEL)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei closed pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
URL: https://github.com/apache/incubator-kyuubi/pull/3609
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992096825
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("version") version: String,
+ @QueryParam("namespace") namespace: String): Response = {
+ val userName = fe.getUserName(Map.empty)
+ val ipAddress = fe.getIpAddress
+ info(s"Received delete engine request from $userName/$ipAddress")
+ if (!userName.equals(administrator)) {
+ throw new NotAllowedException(
+ s"$userName is not allowed to delete kyuubi engine")
+ }
+
+ // validate parameters
+ require(user != null && !user.isEmpty, "user is a required parameter")
Review Comment:
not needed if get user from authentication.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r995260626
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +59,61 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("sharelevel") shareLevel: String,
+ @QueryParam("subdomain") subdomain: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Option(engineType).map(EngineType.withName)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val engineSubdomain = Option(subdomain).filter(_.nonEmpty)
+ .orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ .getOrElse("default")
+ val engineShareLevel = Option(shareLevel).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
Review Comment:
let's refactor it with below.
```
val clonedConf = fe.getConf.clone
Option(engineType).foreach(clonedConf.set(ENGINE_TYPE, _))
clonedConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, Option(subdomain).filter(_.nonEmpty))
Option(shareLevel).filter(_.nonEmpty).foreach(clonedConf.set(ENGINE_SHARE_LEVEL, _))
val normalizedEngineType = clonedConf.get(ENGINE_TYPE)
val engineSubdomain = clonedConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default")
val engineShareLevel = clonedConf.get(ENGINE_SHARE_LEVEL)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992095067
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("version") version: String,
+ @QueryParam("namespace") namespace: String): Response = {
+ val userName = fe.getUserName(Map.empty)
+ val ipAddress = fe.getIpAddress
+ info(s"Received delete engine request from $userName/$ipAddress")
+ if (!userName.equals(administrator)) {
Review Comment:
this api is open for any user.
You can refer https://github.com/apache/incubator-kyuubi/blob/0c2091cd03c92f4b3e402a2dc84ef35cdc5d5bea/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala#L308
to get the userName
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992096178
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("version") version: String,
+ @QueryParam("namespace") namespace: String): Response = {
Review Comment:
```
@QueryParam("hive.server2.proxy.user") hs2ProxyUser: String
```
Support impersonation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992209217
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
Review Comment:
with `Option`, `.filter(_ != null)` is not needed.
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
Review Comment:
Option(engineType)
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
+ userName,
+ Array(normalizedEngineSubdomain))
+
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val serviceNodes = discoveryClient.getServiceNodesInfo(engineSpace)
+ serviceNodes.foreach { node =>
+ val nodePath = s"$engineSpace/${node.nodeName}"
+ info(s"Deleting zookeeper engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete zookeeper engine node:$nodePath", e)
+ throw new NotFoundException(s"Failed to delete zookeeper engine node:$nodePath," +
Review Comment:
remove `zookeeper`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
Review Comment:
Because the value of `fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN)` is Option[String],
so
```
getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
```
->
orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
+ userName,
+ Array(normalizedEngineSubdomain))
+
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val serviceNodes = discoveryClient.getServiceNodesInfo(engineSpace)
Review Comment:
nit: serviceNodes => engineNodes
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
Review Comment:
```
val normalizedEngineType = Option(engineType).map(EngineType.withName).getOrElse(fe.getConf.get(ENGINE_TYPE))
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
Review Comment:
ditto
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
+ userName,
+ Array(normalizedEngineSubdomain))
+
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val serviceNodes = discoveryClient.getServiceNodesInfo(engineSpace)
+ serviceNodes.foreach { node =>
+ val nodePath = s"$engineSpace/${node.nodeName}"
+ info(s"Deleting zookeeper engine node:$nodePath")
Review Comment:
Remove `zookeeper`
```
info(s"Deleting engine node:$nodePath")
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
Review Comment:
```
val engineSubdomain = Option(subdomain).filter(_.nonEmpty).orElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
Review Comment:
Option(engineSubdomain)
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
Review Comment:
Re-order and short the param name, `engineShareLevel` -> `shareLevel`, `engineSubdomain` -> `subdomain`
```
@QueryParam("sharelevel") shareLevel: String,
@QueryParam("subdomain") subdomain: String,
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
Review Comment:
ditto, `Option`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
Review Comment:
```
val engineShareLevel = Option(sharelevel).filter(_.nonEmpty).getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
+ userName,
+ Array(normalizedEngineSubdomain))
+
+ withDiscoveryClient(fe.getConf) { discoveryClient =>
+ val serviceNodes = discoveryClient.getServiceNodesInfo(engineSpace)
+ serviceNodes.foreach { node =>
+ val nodePath = s"$engineSpace/${node.nodeName}"
+ info(s"Deleting zookeeper engine node:$nodePath")
+ try {
+ discoveryClient.delete(nodePath)
+ } catch {
+ case e: Exception =>
+ error(s"Failed to delete zookeeper engine node:$nodePath", e)
Review Comment:
remove `zookeeper`
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
Review Comment:
after shorten the variable name,
```
s"${fe.getConf.get(HA_NAMESPACE)}_${KYUUBI_VERSION}_${engineShareLevel}_${engineType}"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992182091
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/server/api/v1/AdminResourceSuite.scala:
##########
@@ -40,4 +48,49 @@ class AdminResourceSuite extends KyuubiFunSuite with RestFrontendTestHelper {
.post(null)
assert(200 == response.getStatus)
}
+
+ test("delete engine") {
+ val id = UUID.randomUUID().toString
+ conf.set(KyuubiConf.ENGINE_SHARE_LEVEL, USER.toString)
+ conf.set(KyuubiConf.ENGINE_TYPE, SPARK_SQL.toString)
+ conf.set(KyuubiConf.FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
+ conf.set(HighAvailabilityConf.HA_NAMESPACE, "kyuubi_test")
+ conf.set(KyuubiConf.ENGINE_IDLE_TIMEOUT, 180000L)
+ val engine = new EngineRef(conf, Utils.currentUser, id, null)
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"kyuubi_test_${KYUUBI_VERSION}_USER_SPARK_SQL",
+ Utils.currentUser,
+ Array("default"))
+
+ withDiscoveryClient(conf) { client =>
+ engine.getOrCreate(client)
+
+ assert(client.pathExists(engineSpace))
+ var child = client.getChildren(engineSpace)
+ assert(child.size == 1)
+
+ val adminUser = Utils.currentUser
+ val encodeAuthorization = new String(
+ Base64.getEncoder.encode(
+ s"$adminUser:".getBytes()),
+ "UTF-8")
+ val response = webTarget.path("api/v1/admin/engine")
+ .queryParam("user", adminUser)
+ .queryParam("namespace", "kyuubi_test")
+ .queryParam("sharelevel", "USER")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .header(AUTHORIZATION_HEADER, s"BASIC $encodeAuthorization")
+ .delete()
+
+ assert(200 == response.getStatus)
+ assert(client.pathExists(engineSpace))
+ child = client.getChildren(engineSpace)
+ assert(child.size == 0)
+
+ val result = response.readEntity(classOf[String])
+ assert(result.contains(s"Engine ${engineSpace} is deleted successfully."))
+ }
+ }
+
Review Comment:
nit: not needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992228802
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +58,64 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("hive.server2.proxy.user") hs2ProxyUser: String): Response = {
+ val sessionConf = Option(hs2ProxyUser).filter(_.nonEmpty).map(proxyUser =>
+ Map(KyuubiAuthenticationFactory.HS2_PROXY_USER -> proxyUser)).getOrElse(Map())
+
+ var userName: String = null
+ try {
+ userName = fe.getUserName(sessionConf)
+ } catch {
+ case t: Throwable =>
+ throw new NotAllowedException(t.getMessage)
+ }
+
+ // use default value from kyuubi conf when param is not provided
+ val normalizedEngineType = Some(engineType)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_TYPE))
+ val normalizedEngineSubdomain = Some(engineSubdomain)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+ val normalizedEngineShareLevel = Some(engineShareLevel)
+ .filter(_ != null).filter(_.nonEmpty)
+ .getOrElse(fe.getConf.get(ENGINE_SHARE_LEVEL))
+
+ val engineSpace = DiscoveryPaths.makePath(
+ s"${fe.getConf.get(HA_NAMESPACE)}_" +
+ s"${KYUUBI_VERSION}_" +
+ s"${normalizedEngineShareLevel}_${normalizedEngineType}",
Review Comment:
after shorten the variable name,
```
s"${fe.getConf.get(HA_NAMESPACE)}_${KYUUBI_VERSION}_${engineShareLevel}_${normalizedEngineType}"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#issuecomment-1278515742
thanks, merging to master
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3609: [KYUUBI #3410] Support to delete the engine node from engine discovery space with AdminResource
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3609:
URL: https://github.com/apache/incubator-kyuubi/pull/3609#discussion_r992095067
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala:
##########
@@ -53,4 +57,72 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
KyuubiServer.reloadHadoopConf()
Response.ok(s"Refresh the hadoop conf for ${fe.connectionUrl} successfully.").build()
}
+
+ @ApiResponse(
+ responseCode = "200",
+ content = Array(new Content(
+ mediaType = MediaType.APPLICATION_JSON)),
+ description = "delete kyuubi engine")
+ @DELETE
+ @Path("engine")
+ def deleteEngine(
+ @QueryParam("user") user: String,
+ @QueryParam("type") engineType: String,
+ @QueryParam("subdomain") engineSubdomain: String,
+ @QueryParam("sharelevel") engineShareLevel: String,
+ @QueryParam("version") version: String,
+ @QueryParam("namespace") namespace: String): Response = {
+ val userName = fe.getUserName(Map.empty)
+ val ipAddress = fe.getIpAddress
+ info(s"Received delete engine request from $userName/$ipAddress")
+ if (!userName.equals(administrator)) {
Review Comment:
this api is open for any user.
You can refer https://github.com/apache/incubator-kyuubi/blob/0c2091cd03c92f4b3e402a2dc84ef35cdc5d5bea/kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala#L308-L322
to get the userName
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org