Skip to content

Update AdminResource.scala #9

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update AdminResource.scala #9

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import scala.collection.mutable.ListBuffer
import io.swagger.v3.oas.annotations.media.Content
import io.swagger.v3.oas.annotations.responses.ApiResponse
import io.swagger.v3.oas.annotations.tags.Tag
import org.apache.zookeeper.KeeperException.NoNodeException

import org.apache.kyuubi.{KYUUBI_VERSION, Logging, Utils}
import org.apache.kyuubi.client.api.v1.dto.Engine
Expand Down Expand Up @@ -140,9 +141,19 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
}
case None =>
withDiscoveryClient(fe.getConf) { discoveryClient =>
discoveryClient.getChildren(engineSpace).map { child =>
info(s"Listing engine nodes for $engineSpace/$child")
engineNodes ++= discoveryClient.getServiceNodesInfo(s"$engineSpace/$child")
try {
discoveryClient.getChildren(engineSpace).map { child =>
info(s"Listing engine nodes for $engineSpace/$child")
engineNodes ++= discoveryClient.getServiceNodesInfo(s"$engineSpace/$child")
}
} catch {
case nne: NoNodeException =>
error(
s"No such engine for user: $userName, " +
s"engine type: $engineType, share level: $shareLevel, subdomain: $subdomain",
nne)
throw new NotFoundException(s"No such engine for user: $userName, " +
s"engine type: $engineType, share level: $shareLevel, subdomain: $subdomain")
}
}
}
Copy link

Choose a reason for hiding this comment

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

这段代码补丁并没有显著的漏洞风险。对于改进建议，我可能会提出以下几点：

  1. 在捕获异常和处理错误时，应该尽可能具体和明确。在这个例子中，NoNodeException 是一个非常具体的异常，指示 ZooKeeper 的节点不存在。在这里使用它是恰当的，但是我们可能需要更多的上下文信息来确定这是最好的异常，还是应该使用通用的 Exception。

  2. 应考虑在出现错误时提供更详细的日志记录信息。在这个例子中，如果出现 NoNodeException 错误，则会在日志中打印一条信息，并抛出 NotFoundException。这是好的，但更详细的信息有助于诊断和修复错误。

  3. 最后，可以为方法添加测试用例以确保其正确性。

Expand Down