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 #4424][REST] Catch No Node Exception, when list kyuubi engines #4425

Closed
wants to merge 4 commits into from
Closed

[KYUUBI #4424][REST] Catch No Node Exception, when list kyuubi engines #4425

wants to merge 4 commits into from

Conversation

zwangsheng
Copy link
Contributor

Why are the changes needed?

Close #4424

Catch No Node Exception, when list kyuubi engines

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 locally before make a pull request

WX20230227-184957

@zwangsheng zwangsheng changed the title [KYUUBI #4424] Catch No Node Exception, when list kyuubi engines [KYUUBI #4424][REST] Catch No Node Exception, when list kyuubi engines Feb 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2023

Codecov Report

Merging #4425 (3052b15) into master (5aed270) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #4425      +/-   ##
============================================
- Coverage     53.20%   53.17%   -0.04%     
  Complexity       13       13              
============================================
  Files           569      569              
  Lines         31055    31084      +29     
  Branches       4194     4196       +2     
============================================
+ Hits          16522    16528       +6     
- Misses        12973    12993      +20     
- Partials       1560     1563       +3     
Impacted Files Coverage Δ
...rg/apache/kyuubi/server/api/v1/AdminResource.scala 87.39% <100.00%> (-8.17%) ⬇️
...e/kyuubi/jdbc/hive/ClosedOrCancelledException.java 0.00% <0.00%> (-100.00%) ⬇️
...apache/kyuubi/engine/JpsApplicationOperation.scala 77.41% <0.00%> (-3.23%) ⬇️
...in/spark/authz/ranger/SparkRangerAdminPlugin.scala 64.47% <0.00%> (-2.64%) ⬇️
...g/apache/kyuubi/operation/BatchJobSubmission.scala 75.27% <0.00%> (-2.20%) ⬇️
...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala 87.78% <0.00%> (-0.65%) ⬇️
...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala 78.39% <0.00%> (-0.62%) ⬇️

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

@ulysses-you
Copy link
Contributor

Shall we check if engine spec is existed first and return a user-friendly exception ? e.g. No such engine for user: $user, engine type: $engineType, share level: $shareLevel, subdomain: $subdomain

@zwangsheng
Copy link
Contributor Author

zwangsheng commented Feb 28, 2023

Shall we check if engine spec is existed first and return a user-friendly exception ? e.g. No such engine for user: $user, engine type: $engineType, share level: $shareLevel, subdomain: $subdomain

This api used for list all engine under namespace or subdomain.

It‘s reasonable if we return an empty list while simple namespace (like /kyuubi-XXX-version) not created.

Probably not described clearly in desc, No Node Exception is because of the occurrence of kyuubi server namespace node not be created, not specific engine missing.

@zwangsheng
Copy link
Contributor Author

@ulysses-you Throw user-friendly exception when namespace node not created.

@ulysses-you
Copy link
Contributor

thanks, merging to master

@ulysses-you ulysses-you modified the milestones: v1.7.0, v1.8.0 Feb 28, 2023
yanghua pushed a commit to yanghua/incubator-kyuubi that referenced this pull request Apr 13, 2023
…engines

### _Why are the changes needed?_

Close apache#4424

Catch No Node Exception, when list kyuubi engines

### _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

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

![WX20230227-184957](https://user-images.githubusercontent.com/52876270/221544376-2f0d6b4b-0bc4-446e-abb1-d0c79211aea4.png)

Closes apache#4425 from zwangsheng/kyuubi-4424.

Closes apache#4424

3052b15 [zwangsheng] [Kyuubi apache#4424] Fix scala style
74825cf [zwangsheng] [Kyuubi apache#4424] Throw User Friendly Exception
7e8363c [zwangsheng] [Kyuubi apache#4424] Remove usless file & catch subException
4a3c469 [zwangsheng] [Kyuubi apache#4424] Catch cacth No Node Exception, when list kyuubi engines

Authored-by: zwangsheng <[email protected]>
Signed-off-by: ulyssesyou <[email protected]>
yanghua pushed a commit to yanghua/incubator-kyuubi that referenced this pull request Apr 13, 2023
…engines

### _Why are the changes needed?_

Close apache#4424

Catch No Node Exception, when list kyuubi engines

### _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

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

![WX20230227-184957](https://user-images.githubusercontent.com/52876270/221544376-2f0d6b4b-0bc4-446e-abb1-d0c79211aea4.png)

Closes apache#4425 from zwangsheng/kyuubi-4424.

Closes apache#4424

3052b15 [zwangsheng] [Kyuubi apache#4424] Fix scala style
74825cf [zwangsheng] [Kyuubi apache#4424] Throw User Friendly Exception
7e8363c [zwangsheng] [Kyuubi apache#4424] Remove usless file & catch subException
4a3c469 [zwangsheng] [Kyuubi apache#4424] Catch cacth No Node Exception, when list kyuubi engines

Authored-by: zwangsheng <[email protected]>
Signed-off-by: ulyssesyou <[email protected]>
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.

[Bug] [REST] Should cacth NoNode Exception instead of throw, when list kyuubi engines
3 participants