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/07/11 10:53:23 UTC
[GitHub] [incubator-kyuubi] pan3793 opened a new pull request, #3043: Clean up Kyuubi Hive JDBC client
pan3793 opened a new pull request, #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043
<!--
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/contributions.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.
-->
Code clean up
### _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
- [x] [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] pan3793 commented on pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#issuecomment-1182734073
The main purpose of the "code cleanup" is to remove un-referenced code and break nothing, some data structures are shared by client and server, this PR removed the server-specific APIs and code as well.
I'm focused on JDBC standard API, and you are right, some client-side public programing APIs were removed, let me send another PR to restore them.
--
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] pan3793 commented on pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#issuecomment-1182690538
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] yaooqinn commented on a diff in pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#discussion_r919595167
##########
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiQueryResultSet.java:
##########
@@ -318,7 +313,7 @@ public boolean next() throws SQLException {
return false;
}
- /**
+ /*
Review Comment:
nit
--
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] pan3793 commented on a diff in pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#discussion_r920008376
##########
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/KyuubiQueryResultSet.java:
##########
@@ -318,7 +313,7 @@ public boolean next() throws SQLException {
return false;
}
- /**
+ /*
Review Comment:
it's a normal block comment, not java doc
--
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] pan3793 closed pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
pan3793 closed pull request #3043: Clean up Kyuubi Hive JDBC client
URL: https://github.com/apache/incubator-kyuubi/pull/3043
--
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] yaooqinn commented on pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#issuecomment-1182699250
better to let others know what you cleaned, not just `code cleanup`
--
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] yaooqinn commented on a diff in pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#discussion_r919594440
##########
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/JdbcColumn.java:
##########
@@ -353,32 +361,4 @@ static int columnScale(int columnType, JdbcColumnAttributes columnAttributes)
throw new KyuubiSQLException("Invalid column type: " + columnType);
}
}
-
- public Integer getNumPrecRadix() {
Review Comment:
why delete this public API
--
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] yaooqinn commented on pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#issuecomment-1182737417
let's make the PR title and description as clear as possible, especially in the role of Kyuubi PPMC.
--
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] yaooqinn commented on a diff in pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#discussion_r919596061
##########
kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/cli/ColumnBasedSet.java:
##########
@@ -112,43 +75,11 @@ public int numRows() {
return columns.isEmpty() ? 0 : columns.get(0).size();
}
- @Override
- public ColumnBasedSet extractSubset(int maxRows) {
Review Comment:
this is a public programing api...
--
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] codecov-commenter commented on pull request #3043: Clean up Kyuubi Hive JDBC client
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3043:
URL: https://github.com/apache/incubator-kyuubi/pull/3043#issuecomment-1180310520
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3043](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1d1f8fa) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/d3446675ac2cbbc2001325c83e401dd1caf8ab75?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d344667) will **increase** coverage by `0.90%`.
> The diff coverage is `4.50%`.
```diff
@@ Coverage Diff @@
## master #3043 +/- ##
============================================
+ Coverage 50.46% 51.36% +0.90%
Complexity 6 6
============================================
Files 454 452 -2
Lines 25462 25009 -453
Branches 3575 3493 -82
============================================
- Hits 12849 12846 -3
+ Misses 11391 10939 -452
- Partials 1222 1224 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../java/org/apache/kyuubi/jdbc/KyuubiHiveDriver.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvS3l1dWJpSGl2ZURyaXZlci5qYXZh) | `4.87% <0.00%> (+0.22%)` | :arrow_up: |
| [...uubi/jdbc/hive/HttpKerberosRequestInterceptor.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9IdHRwS2VyYmVyb3NSZXF1ZXN0SW50ZXJjZXB0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...e/kyuubi/jdbc/hive/HttpRequestInterceptorBase.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9IdHRwUmVxdWVzdEludGVyY2VwdG9yQmFzZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/kyuubi/jdbc/hive/HttpTokenAuthInterceptor.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9IdHRwVG9rZW5BdXRoSW50ZXJjZXB0b3IuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...n/java/org/apache/kyuubi/jdbc/hive/JdbcColumn.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9KZGJjQ29sdW1uLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/kyuubi/jdbc/hive/JdbcTable.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9KZGJjVGFibGUuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...g/apache/kyuubi/jdbc/hive/KyuubiBaseResultSet.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9LeXV1YmlCYXNlUmVzdWx0U2V0LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../org/apache/kyuubi/jdbc/hive/KyuubiConnection.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9LeXV1YmlDb25uZWN0aW9uLmphdmE=) | `4.17% <0.00%> (+0.04%)` | :arrow_up: |
| [.../org/apache/kyuubi/jdbc/hive/KyuubiDataSource.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9LeXV1YmlEYXRhU291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pache/kyuubi/jdbc/hive/KyuubiDatabaseMetaData.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9LeXV1YmlEYXRhYmFzZU1ldGFEYXRhLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [28 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [1bc3916...1d1f8fa](https://codecov.io/gh/apache/incubator-kyuubi/pull/3043?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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