Skip to content

chore(clickhouse): bump clickhouse-jdbc to v0.6.0 (latest)#21894

Closed
hainenber wants to merge 1 commit intoprestodb:masterfrom
hainenber:bump-clickhouse-jdbc-0.6.0
Closed

chore(clickhouse): bump clickhouse-jdbc to v0.6.0 (latest)#21894
hainenber wants to merge 1 commit intoprestodb:masterfrom
hainenber:bump-clickhouse-jdbc-0.6.0

Conversation

@hainenber
Copy link
Copy Markdown
Contributor

@hainenber hainenber commented Feb 9, 2024

Description

Bump clickhouse-jdbc to latest version, 0.6.0. The groupID has been changed so there are refactoring accordingly.

Motivation and Context

Closes #21846

Impact

No impact

Test Plan

Use CI to ensure no breakages

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Upgrade clickhouse-jdbc to v0.6.0

@hainenber hainenber requested a review from a team as a code owner February 9, 2024 16:52
@hainenber hainenber requested a review from presto-oss February 9, 2024 16:52
@steveburnett
Copy link
Copy Markdown
Contributor

Following the release notes guidelines, suggest revising the release note entry. Suggested:

== RELEASE NOTES ==

Security Changes
* Upgrade clickhouse-jdbc to v0.6.0

@hainenber
Copy link
Copy Markdown
Contributor Author

Thanks Steve for the callout, appreciate it!

@agrawalreetika
Copy link
Copy Markdown
Member

agrawalreetika commented Feb 11, 2024

Hi @hainenber
Thank you for the PR.
It looks to me that clickhouse tests are not enabled in Circle CI. I have tried to run exiting clickhouse tests manually on local with your change, which seems to be failing with below error:

2024-02-11T05:07:36.802-0600 INFO Waiting for database connection to become available at jdbc:clickhouse://localhost:49468/default using query 'SELECT 1'
2024-02-11T05:07:36.944-0600 WARNING Error when creating APACHE_HTTP_CLIENT, fall back to HTTP_URL_CONNECTION
java.lang.NoClassDefFoundError: org/apache/hc/client5/http/io/HttpClientConnectionManager

Could you please confirm how did you test your change?

@tdcmeehan Could you please suggest if we are open to enable clickhouse tests in github actions or if there is any reason we have not enabled it?

@hainenber
Copy link
Copy Markdown
Contributor Author

That was my mistake 😵‍💫 . I was assuming this is covered by the CI and didn't run presto-clickhouse tests locally. I'll try to replicate your error locally and revert soon!

@hainenber
Copy link
Copy Markdown
Contributor Author

Hi @agrawalreetika, I've ran Maven's test lifecycle on presto-clickhouse for this change but to my surprise, the job completes without any issues!

image

Is there any info that can assist me reproducing your issue locally?

@agrawalreetika
Copy link
Copy Markdown
Member

Looks like in clickhouse connector pom TestClickHouseDistributedQueries is excluded so it didn't run as part of Tests. It just ran 3 tests which are in TestClickHouseConfig & TestClickHousePlugin

Try running Query tests in TestClickHouseDistributedQueries

@hainenber hainenber closed this Mar 8, 2024
@hainenber hainenber deleted the bump-clickhouse-jdbc-0.6.0 branch March 8, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade clickhouse-jdbc to latest 0.6.0

3 participants