Skip to content

feat: add Apache Kafka driver - #93

Open
Siyet wants to merge 3 commits into
trunkfrom
12-kafka-driver
Open

feat: add Apache Kafka driver#93
Siyet wants to merge 3 commits into
trunkfrom
12-kafka-driver

Conversation

@Siyet

@Siyet Siyet commented Apr 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • New KafkaDriver on top of kafkajs (pure JS, no native deps). Lazy-required inside connect() so non-kafka users never pay.
  • Schema tree renders clustertopics / consumer groups; each topic carries its partitions as children. Tree icon falls back to codicon broadcast (no glyph in the viewstor icon font yet).
  • Small command DSL via parseKafkaCommand(): LIST TOPICS, LIST GROUPS, DESCRIBE <topic>, CONSUME <topic> [N] [from-beginning], PRODUCE <topic> [key] <value>. PRODUCE is blocked on read-only connections. getTableData(topic) = CONSUME <topic> with up to MAX_RESULT_ROWS messages.
  • Connection form gets an "Apache Kafka" option. Username/password map to SASL/PLAIN (options.saslMechanism overrides to scram-sha-256/scram-sha-512). Host accepts a comma-separated bootstrap broker list. SSL and Proxy/Advanced sections hidden for kafka.

Closes #12

Assumptions

  • Scope narrowed to the core driver + browse/consume/produce. The issue lists additional items (e2e via testcontainers, "consumer groups as a separate node" detail view, topic configs from describeConfigs, message-browser JSON expansion polish, safe mode for PRODUCE outside readonly). Shipping the driver first lets us react to real usage — each of the remaining items slots in without changing the driver interface. The current minimum is enough to register Kafka connections, browse the cluster, and run round-trips.
  • getTableData(topic) uses a throwaway consumer group. viewstor-<ts>-<rand> with a 5-second max wait and fromBeginning: false, so opening a topic shows "recent activity". Asking for history goes through CONSUME <topic> N from-beginning explicitly. This matches the issue's "consume last N messages" framing and keeps the UI fast on high-throughput topics.
  • No getDDL / getCompletions / getIndexedColumns / getTableStatistics. Topics aren't SQL and autocomplete / DDL / index hints don't have a natural mapping. driverContract.test.ts explicitly asserts the empty optional set so future additions force an update here.
  • No MCP / chart changes. Kafka connections show up through list_connections because that tool is driver-agnostic, and execute_query accepts the DSL strings as-is. build_chart over topic data works once a user opens a topic and pins the consume result — covered by existing MCP plumbing, no code change needed.
  • No new l10n strings. Added one English-only hint block ("For multi-broker clusters…") and reused the existing form labels. Will extract into package.nls.json if this ships.

Follow-ups (not in this PR, tracked under #12)

  • e2e tests with testcontainers (confluentinc/cp-kafka).
  • Topic config view (describeConfigs: retention, cleanup policy) under Show Table Info.
  • JSON value expansion in the Result Panel for message value cells.
  • Long-running consumer view that streams live messages instead of sampling.
  • viewstor-kafka glyph in the viewstor icon font.

Manual test cases

  1. Golden path — list topics. docker run -p 9092:9092 apache/kafka:3.8.0 (or any single-broker cluster). New Connection → type Apache Kafka → Host localhost, Port 9092. Test connection → green. Save, expand the tree. Expect: clustertopics with any pre-existing topics, each expandable to partitions.
  2. LIST TOPICS / CONSUME via editor. Right-click the connection → Open Query. Run LIST TOPICS; → result grid shows topic | partitions. Run CONSUME __consumer_offsets 10 from-beginning → 10 rows with offset, partition, key, value, timestamp, headers.
  3. PRODUCE round-trip. Run PRODUCE demo hello "world"produced 1 message to demo. Then CONSUME demo from-beginning → your message appears.
  4. Read-only blocks PRODUCE. Edit the connection, tick "Read-only". Retry PRODUCE demo hello world → error PRODUCE is not allowed on a read-only Kafka connection. CONSUME still works.
  5. Multi-broker bootstrap. Set Host to broker1:9092, broker2:9092. Test connection. Expect: kafkajs connects through whichever broker responds first; same tree output.
  6. SASL/PLAIN. Against a SASL-secured cluster, set username/password + enable SSL. Expect: connect succeeds; disabling SSL against a TLS listener fails with a clean error in the testResult banner.
  7. DESCRIBE. DESCRIBE demo → one row per partition with leader, replicas, isr JSON.
  8. Unknown command. SELECT 1 in a kafka editor → error Unknown Kafka command: SELECT. No broker traffic.
  9. Schema cache invalidation. Create a new topic via an external tool → right-click connection → Refresh. Expect: new topic appears in the tree without reconnecting.

Checklist

  • CHANGELOG.md[Unreleased] / Added entry with (#12) link
  • CLAUDE.mdkafka.ts (kafkajs) added to the Drivers line plus a new Kafka-driver paragraph documenting the DSL, schema shape, and SASL wiring
  • README.md — tagline updated ("PostgreSQL + Redis + ClickHouse + SQLite + Kafka…"), comparison table, Supported Databases table row
  • Unit testssrc/test/kafka.test.ts (24 tests): parseKafkaCommand verb + option handling, parseBrokerList with/without port and with comma-separated lists, normalizeHeaders for Uint8Array/null/string/number
  • driverContract.test.ts — new KafkaDriver spec with empty expectedOptional set; kafkajs mocked alongside the other native modules
  • l10n — no new entries; the one added hint string ("For multi-broker clusters…") stays English for this PR and will migrate to package.nls.json when the first non-English string lands
  • Wiki — follow-up "Kafka driver" page (DSL cheat sheet, SASL mechanisms, consumer-group cleanup). Not pushed from this PR.

claude added 2 commits April 15, 2026 21:22
Introduces a KafkaDriver on top of `kafkajs` (pure JS, no native deps) so
users can browse topics, consume and produce messages without leaving
VS Code. The schema tree shows a single `cluster` keyspace with `topics`
and `consumer groups` sections; topics expose their partitions as
children and "Show Messages" / `CONSUME` render the last N records in
the Result Panel.

Query DSL parsed by `parseKafkaCommand`:
  LIST TOPICS | LIST GROUPS
  DESCRIBE <topic>
  CONSUME <topic> [N] [from-beginning]
  PRODUCE <topic> [key] <value>

`PRODUCE` is blocked on read-only connections. SASL/PLAIN is wired via
username/password; `options.saslMechanism` overrides the mechanism.
The Host field accepts a comma-separated bootstrap broker list.
- subscribe({ topics: [topic] }) to use kafkajs 2.x shape and silence
  the deprecation warning emitted on every consume.
- parseBrokerList falls back to localhost:<port> when the host string
  trims/splits to zero non-empty parts (previously produced an empty
  brokers array → opaque kafkajs error).
- normalizeHeaders handles multi-valued (array) header values and drops
  keys whose values are all null/undefined.
- getTableInfo no longer reports partition count as the topic row
  count; topics have no meaningful row count.

https://claude.ai/code/session_019u3fc2f7V6FDVUwZET3Vub

@Siyet Siyet left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixes pushed

  • 1bb3b99consumer.subscribe({ topics: [topic] }) uses the kafkajs 2.x signature; the old { topic } shape logs a deprecation warning on every consume.
  • 1bb3b99parseBrokerList falls back to localhost:<port> when the trimmed/split broker list is empty (e.g. " " or ", ,"). Previously produced an empty brokers array → opaque kafkajs error.
  • 1bb3b99normalizeHeaders handles multi-valued (array) header values and drops keys whose values are all null/undefined. kafkajs IHeaders allows arrays; the old path stringified them to "[object Object]".
  • 1bb3b99getTableInfo no longer reports partition count as rowCount; topics have no meaningful row count and reporting partitions is misleading downstream.

Issues to address

  • src/drivers/kafka.ts:258-295consumeMessages creates an ephemeral viewstor-<ts>-<rand> consumer group every call and never removes it. With many previews these groups accumulate on the broker until the cluster's offsets.retention.minutes kicks in (7 days by default). Call this.admin.deleteGroups([groupId]) after consumer.disconnect() in a swallowed try/catch.
  • src/drivers/kafka.ts:189-225getSchema surfaces internal topics (__consumer_offsets, __transaction_state, _schemas) alongside user topics with no way to collapse them. Filter t.name.startsWith('_') into a separate "Internal topics" group, or at minimum hide by default — the tree quickly becomes noisy on any real cluster.
  • src/drivers/kafka.ts:247-256getTableData ignores offset and orderBy from the driver interface. Result Panel pagination (next page / sort columns) silently re-fetches the most recent N messages every time. Either short-circuit pagination in the Result Panel for Kafka connections, or surface the limitation (e.g. a banner "Kafka topics are non-paginated — use CONSUME <topic> N from-beginning for historical data").
  • src/drivers/kafka.ts:156-177 — PRODUCE is not routed through Safe Mode, yet it's a write-like operation. safeMode: 'block' / 'warn' on a Kafka connection has no effect; users expecting the same guardrails as SQL writes will be surprised. Either wire Safe Mode or document that it is intentionally SQL-only.
  • src/drivers/kafka.ts:266-289 — the 5s timer resolve() callback doesn't clearTimeout or short-circuit further eachMessage invocations; the consumer keeps pushing into collected (no-oped by the cap check) until disconnect() returns. Minor, but sets collected.push work on the critical path after we've already returned to the caller.
  • src/test/kafka.test.ts — no coverage for the CONSUME limit clamp (cap = Math.max(1, Math.min(limit, MAX_RESULT_ROWS))), e.g. CONSUME t 0 and CONSUME t 99999999. Worth one it(...) since the clamp is the only guard against a user-supplied limit blowing past MAX_RESULT_ROWS.

CI note

E2E Tests job on 24483389082 failed in ~56s (Lint / Unit / Build / Audit / Changeset all pass). The failure appears unrelated to this PR — no e2e file references the Kafka driver, and the e2e suite doesn't import createDriver. Local vitest run src/test/e2e/sqlite.e2e.test.ts passes. Likely a testcontainers / Docker Hub flake on the runner; re-run should clear it. Logs were not accessible to this review.


Generated by Claude Code

@Siyet Siyet added reviewed and removed reviewed labels Apr 15, 2026
If subscribe/run rejects, the existing code skipped the disconnect call and
leaked a connected consumer plus its throwaway group on the broker. Wrap
post-connect logic in try/finally and clear the 5s timeout on the reject
path so it does not linger.

@Siyet Siyet left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixes pushed

  • 8519d0e — Always disconnect the Kafka consumer on error paths. The previous code ran subscribe() and the consume Promise outside any try/finally, so a broker-side failure would skip consumer.disconnect() and leak a connected consumer plus its throwaway group (only cleaned up after sessionTimeout). Also clears the 5s timer on the run().catch(reject) path so it doesn't linger after a reject.

Issues to address

  • src/commands/tableCommands.ts:50 — Right-clicking a Kafka topic → Show Table Data opens the Result Panel with readonly derived from isConnectionReadonly(id). For a non-readonly kafka connection, the UI exposes row-edit/insert/delete buttons whose _saveEditsbuildUpdateSql output gets rejected by parseKafkaCommand as Unknown Kafka command: UPDATE. Either force readonly = true when config.type === 'kafka', or hide the edit controls when the driver can't satisfy them.
  • src/drivers/kafka.ts:38this.readonlyMode = !!config.readonly only reflects the connection's own flag, not the folder-inherited readonly that ConnectionManager.isConnectionReadonly() computes. A connection inside a readonly folder that doesn't set its own flag will still run PRODUCE. Same pattern exists in other drivers, but it matters more here because PRODUCE is the driver's only write path and nothing above the driver gates it.
  • src/drivers/kafka.ts:256consumeMessages creates a fresh throwaway consumer group on every call. For getTableData (tree → Show Table Data) this hides the pagination semantics: pages >1 just re-consume from latest/from-beginning again instead of continuing from the previous offset, so the Result Panel's pager is a no-op. Either accept a starting offset and seek, or wire the Result Panel to disable paging for kafka.
  • src/drivers/kafka.ts:70ping() calls this.admin.listTopics() without checking whether admin is defined. After disconnect() sets admin = undefined, a ping returns false via the catch block but the underlying TypeError gets swallowed silently — fine for the current callers, worth a guard if ping starts being used for liveness after disconnect.
  • src/test/kafka.test.ts — Coverage is solid for parseKafkaCommand / parseBrokerList / normalizeHeaders, but there's no test for the consumer lifecycle (cap reached, timeout, reject path). A fake consumer that invokes eachMessage N times and one that rejects run() would catch regressions in the new try/finally and the timer/reject cleanup.

Generated by Claude Code

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add Apache Kafka driver

2 participants