-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[hive] add hive client pool #3004
Conversation
hi @zhuangchong Is there any difference between this PR and #2567 ?The e2e test has been unstable before. |
There is no difference. PR 2567 has not been updated for a long time, and this feature is very needed, so I launched a PR again, merging the latest code and solving the UT exception. If you have time and are willing to update PR 2567, I can close this PR. @XuQianJin-Stars |
I understand. I don't care about this PR merge. I will close PR 2567. The reason I left a comment is to tell me you want to use this PR 2567. |
hi @zhuangchong I think it's ok to add XuQianJin-Stars as co-author |
Where is this added? |
For this situation, I think you can make changes based on the original pr and then push the modified code to the original author's repository |
@zhuangchong Add co-author information in the commit message, for example: |
217ad91
to
355641a
Compare
Hi @zhuangchong |
@JingsongLi I merged the latest code, and then ut error package cannot be found, I am looking for the problem. |
@zhuangchong You mean you just cherry-picked #2567 ? Why not kust keep your code? |
a4d3899
to
47409b1
Compare
The CI is experiencing anomalies after merging the latest code into the master branch. |
Is CachedClientPool considering creating an abstraction layer? JdbcClientPool may also need to be implemented through caching |
Sorry for the late reply. I've been thinking for a long time, why do we need a client pool? Are there any user needs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Purpose
add hive client pool
Tests
API and Format
Documentation