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 #2008][FOLLOWUP] Support engine type and subdomain in kyuubi-ctl #2233

Closed
wants to merge 1 commit into from

Conversation

cxzl25
Copy link
Contributor

@cxzl25 cxzl25 commented Mar 27, 2022

Why are the changes needed?

#2008

In version 1.5, the kyuubi version is added to the engine path registered to the znode, which makes the original list engine operation invalid.

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

@cxzl25
Copy link
Contributor Author

cxzl25 commented Mar 27, 2022

Fix

image

@codecov-commenter
Copy link

Codecov Report

Merging #2233 (78b8336) into master (a5b4c1b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #2233   +/-   ##
=========================================
  Coverage     61.80%   61.81%           
  Complexity       69       69           
=========================================
  Files           331      331           
  Lines         16021    16021           
  Branches       2031     2031           
=========================================
+ Hits           9902     9903    +1     
+ Misses         5297     5295    -2     
- Partials        822      823    +1     
Impacted Files Coverage Δ
...cala/org/apache/kyuubi/ctl/ServiceControlCli.scala 77.57% <100.00%> (ø)
...ache/kyuubi/operation/KyuubiOperationManager.scala 94.11% <0.00%> (-1.97%) ⬇️
...rg/apache/kyuubi/engine/trino/TrinoStatement.scala 69.87% <0.00%> (+2.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5b4c1b...78b8336. Read the comment docs.

@ulysses-you
Copy link
Contributor

thanks, merging to master/branch-1.5

ulysses-you pushed a commit that referenced this pull request Mar 28, 2022
### _Why are the changes needed?_
#2008

In version 1.5, the kyuubi version is added to the engine path registered to the znode, which makes the original list engine operation invalid.

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

- [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #2233 from cxzl25/KYUUBI-2008-FOLLOWUP.

Closes #2008

78b8336 [sychen] Engine space with kyuubi version

Authored-by: sychen <sychen@trip.com>
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
(cherry picked from commit 015cbe5)
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
@ulysses-you ulysses-you added this to the v1.5.1 milestone Mar 28, 2022
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.

3 participants