david-streamlio commented on PR #33: URL: https://github.com/apache/pulsar-connectors/pull/33#issuecomment-4681513938
Thanks for the fix, @jknetl — the driver bump looks correct for #32. One thing to flag before merge: CI hasn't actually run on this PR yet. The "Pulsar Connectors CI" workflow is sitting in the action_required state (pending maintainer approval for a first-time contributor), so neither the build nor the unit test job has executed. A maintainer will need to click "Approve and run workflows" so we get a green run before merging. A couple of caveats worth noting even once CI passes: - The jdbc/clickhouse module has no src/test, and there are no ClickHouse integration tests in the repo. The CI matrix only runs ./gradlew test (no container-based integration job), so a green build won't actually exercise this driver against ClickHouse. - The bug in #32 (Not able to find table: pulsar_messages) is a runtime driver-compatibility issue that unit tests won't catch. Could you confirm you've manually verified the sink against a recent ClickHouse version with this 0.9.8 driver? - This is a large jump (0.4.6 → 0.9.8). The ClickHouse Java client reorganized its artifacts/packages across the 0.5–0.7 line, so it'd be good to confirm the `com.clickhouse:clickhouse-jdbc` coordinates and classes still resolve cleanly at build time. Once CI is approved/green and you can confirm a manual ClickHouse check, this is good to go from my side. 👍 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
