Skip to content

Document Karafka::Admin.new(external_client:), fix Configuration Scope claim - #990

Open
coipond-writer[bot] wants to merge 1 commit into
masterfrom
fix/admin-api-external-client-undocumented-44801
Open

Document Karafka::Admin.new(external_client:), fix Configuration Scope claim#990
coipond-writer[bot] wants to merge 1 commit into
masterfrom
fix/admin-api-external-client-undocumented-44801

Conversation

@coipond-writer

Copy link
Copy Markdown
Contributor

What

`Infrastructure/Admin/API.md`'s "Configuration Scope" note said "Only the `kafka` configuration is customizable per Admin instance."

Why it's wrong

`karafka/lib/karafka/admin.rb:55`: `def initialize(kafka: {}, external_client: nil)`. `external_client:` is a 2.6.0 addition letting admin operations run on an already-open client (a raw rdkafka admin instance, one wrapped with `Connection::Proxy`, or a `Connection::Client` of a running consumer) instead of opening a new short-lived connection. Its lifecycle stays fully with its owner. This directly contradicts the "only `kafka`" claim and was undocumented anywhere on the wiki.

Fix

Corrected the note and added a short subsection documenting `external_client:`, framed per the source docstring as a low-level API.

Fixes #44801 (Redmine).

…e claim

karafka/lib/karafka/admin.rb:55: def initialize(kafka: {}, external_client: nil).
external_client: is a 2.6.0 addition letting admin operations run on an
already-open client (a raw rdkafka admin instance, one wrapped with
Connection::Proxy, or a Connection::Client of a running consumer)
instead of opening a new short-lived connection. Its lifecycle stays
fully with its owner.

The "Configuration Scope" note claimed kafka: was the only customizable
per-instance argument, directly contradicting this. Fixed the note and
added a short subsection documenting external_client:, framed per the
source docstring as a low-level API.

Fixes #44801.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

0 participants