Skip to content

Docs: fix issue with client properties name in Kafka connect sink docs - #113236

Merged
Blargian merged 1 commit into
ClickHouse:masterfrom
chernser:docs/kafka_update_config
Aug 4, 2026
Merged

Docs: fix issue with client properties name in Kafka connect sink docs#113236
Blargian merged 1 commit into
ClickHouse:masterfrom
chernser:docs/kafka_update_config

Conversation

@chernser

@chernser chernser commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Updates Kafka Connect Sink documentation where wrongly mentioned that clickhouseSettings should be used for client config (socket_timeout and connection_timeout) while all client settings are passed via jdbcConnectionProperties

Changelog category (leave one):

  • Documentation (changelog entry is not required)

Version info

  • Merged into: 26.8.1.744 (included in 26.8 and later)

@chernser
chernser requested a review from a team as a code owner August 4, 2026 06:30
@clickhouse-gh

clickhouse-gh Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [fd26b6d]

Summary:


AI Review

Summary

This PR updates the Kafka Connect Sink docs to move client timeout settings from clickhouseSettings to jdbcConnectionProperties, which is the right direction. I would still block merge, because the page does not yet show a valid jdbcConnectionProperties example and it remains self-contradictory about where socket_timeout belongs.

Findings

⚠️ Majors

  • [docs/integrations/connectors/data-ingestion/kafka/kafka-clickhouse-connect-sink.mdx:865] The new example still formats jdbcConnectionProperties incorrectly by comma-separating the parameters, even though the same page documents this field as a ?key=value&key2=value2 query string. The later high-throughput recipe at docs/integrations/connectors/data-ingestion/kafka/kafka-clickhouse-connect-sink.mdx:937 also still routes socket_timeout through clickhouseSettings. As a result, the PR leaves readers with conflicting and partially non-working configuration guidance.
    Suggested fix: change the timeout example to "?socket_timeout=300000&connection_timeout=30000" and add/move jdbcConnectionProperties in the later recipe instead of keeping socket_timeout inside clickhouseSettings.
Final Verdict

Changes requested.

@clickhouse-gh clickhouse-gh Bot added the pr-documentation Documentation PRs for the specific code PR label Aug 4, 2026
@Blargian Blargian added the can be tested Allows running workflows for external contributors label Aug 4, 2026
@Blargian Blargian self-assigned this Aug 4, 2026
@Blargian Blargian changed the title Fixed issue with client properties name in Kafka connect sink docs Docs: fix issue with client properties name in Kafka connect sink docs Aug 4, 2026
@Blargian
Blargian added this pull request to the merge queue Aug 4, 2026

```json
"clickhouseSettings": "socket_timeout=300000,connection_timeout=30000"
"jdbcConnectionProperties": "?socket_timeout=300000,connection_timeout=30000"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jdbcConnectionProperties is documented earlier on this page as a URL query string that starts with ? and joins pairs with &, so this replacement is still not a working example: "?socket_timeout=300000,connection_timeout=30000" keeps the old comma-separated syntax. The page also still has the high-throughput recipe at docs/integrations/connectors/data-ingestion/kafka/kafka-clickhouse-connect-sink.mdx:937 putting socket_timeout into clickhouseSettings, so readers will still see conflicting guidance about where client timeouts belong.

Can we switch this example to "?socket_timeout=300000&connection_timeout=30000" and move the later socket_timeout into jdbcConnectionProperties as well?

Merged via the queue into ClickHouse:master with commit 1e26bff Aug 4, 2026
526 of 530 checks passed
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

can be tested Allows running workflows for external contributors pr-documentation Documentation PRs for the specific code PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants