-
Notifications
You must be signed in to change notification settings - Fork 938
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
Fix ClickHouse integration tests #6915
Conversation
Semms downgrading ClickHouse Server version breaks some metadata queries ... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6915 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 688 688
Lines 42590 42590
Branches 5805 5805
======================================
Misses 42590 42590 ☔ View full report in Codecov by Sentry. |
Pin |
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.
LGTM
### Why are the changes needed? I observed ClickHouse integration test failure in GHA, after some investigation, the root cause is testcontainers/testcontainers-java#9942 ``` /entrypoint.sh: neither CLICKHOUSE_USER nor CLICKHOUSE_PASSWORD is set, disabling network access for user 'default' ``` In short, the recent ClickHouse docker image does not allow the `default` user to connect without a password, unfortunately, `testcontainers-scala-clickhosue` does not expose API to set CLICKHOSUE_USER and CLICKHOUSE_PASSWORD, as a workaround, I pin `clickhouse-server:24.3.15`(the latest version has no such restriction) until a fixed version of Testcontainers available. This PR also switches the `clickhouse-jdbc`'s classifier from `http` to `shaded`, the reason is, `http` does not ship ApacheHttpClient5, previously, it happened to work because `iceberg-runtime-spark3.5_2.12` packaged un-relocated ApacheHttpClient5 classes, but it gets fixed in Iceberg 1.8.0, then `clickhouse-jdbc:http` stop working. ``` java.lang.NoClassDefFoundError: org/apache/hc/core5/http/HttpRequest ``` Additionally, this PR bumps `clickhouse-jdbc` from 0.6.0 to 0.6.5. ### How was this patch tested? Pass GHA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #6915 from pan3793/fix-ch-test. Closes #6915 996f095 [Cheng Pan] Pin clickhouse-server:24.3.15 d633df0 [Cheng Pan] Bump clickhouse-jdbc 0.6.5 214c8a2 [Cheng Pan] Fix ClickHouse integration tests Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit d49c631) Signed-off-by: Cheng Pan <[email protected]>
Thanks, merged to master/1.10 |
Why are the changes needed?
I observed ClickHouse integration test failure in GHA, after some investigation, the root cause is testcontainers/testcontainers-java#9942
In short, the recent ClickHouse docker image does not allow the
default
user to connect without a password, unfortunately,testcontainers-scala-clickhosue
does not expose API to set CLICKHOSUE_USER and CLICKHOUSE_PASSWORD, as a workaround, I pinclickhouse-server:24.3.15
(the latest version has no such restriction) until a fixed version of Testcontainers available.This PR also switches the
clickhouse-jdbc
's classifier fromhttp
toshaded
, the reason is,http
does not ship ApacheHttpClient5, previously, it happened to work becauseiceberg-runtime-spark3.5_2.12
packaged un-relocated ApacheHttpClient5 classes, but it gets fixed in Iceberg 1.8.0, thenclickhouse-jdbc:http
stop working.Additionally, this PR bumps
clickhouse-jdbc
from 0.6.0 to 0.6.5.How was this patch tested?
Pass GHA.
Was this patch authored or co-authored using generative AI tooling?
No.