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 #4216] Support to transfer client version for kyuubi hive jdbc and rest client sdk #4370

Closed
wants to merge 2 commits into from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Feb 19, 2023

Why are the changes needed?

To close #4216

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

@turboFei turboFei changed the title Client version Support to transfer client version for jdbc connection and rest client sdk Feb 19, 2023
@turboFei turboFei changed the title Support to transfer client version for jdbc connection and rest client sdk Support to transfer client version for kyuubi hive jdbc and rest client sdk Feb 19, 2023
@turboFei turboFei changed the title Support to transfer client version for kyuubi hive jdbc and rest client sdk [KYUUBI #4216] Support to transfer client version for kyuubi hive jdbc and rest client sdk Feb 19, 2023
@turboFei
Copy link
Member Author

cc @pan3793 @lightning-L

@turboFei turboFei force-pushed the client_version branch 2 times, most recently from 8db8b34 to 2a445f3 Compare February 19, 2023 13:42
@turboFei turboFei requested a review from pan3793 February 19, 2023 15:05
@turboFei turboFei added this to the v1.8.0 milestone Feb 19, 2023
@turboFei turboFei self-assigned this Feb 19, 2023
@turboFei
Copy link
Member Author

turboFei commented Feb 19, 2023

image

The ut passed locally, I do not know why it failed on GA, it does not make senses.

@pan3793

add resources

save

save

save

save

save

save

nit

nit

fix

fix and remove unused

Revert "fix and remove unused"

This reverts commit 15e4465.

fix

nit
@turboFei
Copy link
Member Author

move the UT to perConnectionSuite.

Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

Codecov Report

Merging #4370 (efd934c) into master (5b2c406) will decrease coverage by 0.01%.
The diff coverage is 68.96%.

@@             Coverage Diff              @@
##             master    #4370      +/-   ##
============================================
- Coverage     53.68%   53.67%   -0.01%     
  Complexity       13       13              
============================================
  Files           562      563       +1     
  Lines         30834    30863      +29     
  Branches       4157     4157              
============================================
+ Hits          16552    16566      +14     
- Misses        12731    12742      +11     
- Partials       1551     1555       +4     
Impacted Files Coverage Δ
.../org/apache/kyuubi/config/KyuubiReservedKeys.scala 0.00% <ø> (ø)
.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java 5.75% <0.00%> (-0.01%) ⬇️
...va/org/apache/kyuubi/client/util/VersionUtils.java 63.63% <63.63%> (ø)
...c/main/java/org/apache/kyuubi/jdbc/hive/Utils.java 41.06% <66.66%> (+1.16%) ⬆️
...in/java/org/apache/kyuubi/client/BatchRestApi.java 97.56% <87.50%> (-2.44%) ⬇️
...ache/kyuubi/server/mysql/MySQLCommandHandler.scala 79.79% <0.00%> (-2.03%) ⬇️
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala 59.09% <0.00%> (-1.52%) ⬇️
...mon/src/main/scala/org/apache/kyuubi/Logging.scala 41.25% <0.00%> (-1.25%) ⬇️
...che/kyuubi/server/KyuubiTHttpFrontendService.scala 60.00% <0.00%> (-0.69%) ⬇️
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 97.45% <0.00%> (-0.07%) ⬇️
... and 1 more

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

@pan3793
Copy link
Member

pan3793 commented Feb 20, 2023

we can add a udf kyuubi_client_version() in the followup PR.

@turboFei turboFei modified the milestones: v1.8.0, v1.7.0 Feb 20, 2023
@turboFei
Copy link
Member Author

thanks, merging to master and 1.7

@turboFei turboFei closed this in 3f6ba77 Feb 20, 2023
turboFei added a commit that referenced this pull request Feb 20, 2023
…c and rest client sdk

### _Why are the changes needed?_

To close #4216

### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

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

Closes #4370 from turboFei/client_version.

Closes #4216

efd934c [fwang12] save
cf8acd5 [fwang12] version properties

Authored-by: fwang12 <fwang12@ebay.com>
Signed-off-by: fwang12 <fwang12@ebay.com>
(cherry picked from commit 3f6ba77)
Signed-off-by: fwang12 <fwang12@ebay.com>
@turboFei turboFei deleted the client_version branch February 20, 2023 03:18
@turboFei
Copy link
Member Author

we can add a udf kyuubi_client_version() in the followup PR.

cc @lightning-L

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.

[Improvement] Record the client version both for interactive and batch session
4 participants