Repository navigation
Conversation
Adds `KafkaDataReader` and `KafkaDataWriter` on the `MessageDataReader` / `MessageDataWriter` bases: - Consumer uses a consumer group with auto-commit disabled; acknowledging a batch commits the offsets following the processed records, highest per partition, so records still buffered remain eligible for redelivery. - Producer submits a batch's records together and flushes once, keeping the client's internal batching instead of one round trip per record. - `bootstrap_servers` falls back to `KAFKA_BOOTSTRAP_SERVERS` and accepts a string or a list; aiokafka failures are mapped onto the broker exception hierarchy (authorization / record-too-large are permanent, connection and rebalance errors are retried). - The `kafka` extra declares aiokafka, and aiokafka is in the test dependency group so the unit tests can import the real error and offset types. 372 lines of tests cover offset commits (including out-of-order records across partitions), empty polls returning no messages, multi-partition flattening, batched sends with a single flush, error mapping, settings resolution and JSON vs raw encoding. Split out of #284, where it landed with the PubSub and SQS/SNS implementations.
|
Benchmark comparison for |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it adds
KafkaDataReaderandKafkaDataWriteron theMessageDataReader/MessageDataWriterbases from #284 - the Apache Kafka equivalent of the PubSub and SQS/SNS components.enable_auto_commit=False; acknowledging a batch commitsTopicPartition -> OffsetAndMetadata(offset + 1)for the records actually processed, taking the highest offset per partition, so records still waiting in the buffer remain eligible for redelivery. An emptygetmanypoll returns no messages rather than ending the stream, matching the base class's "idle is not exhaustion" rule.send()for every record and flushed once, so aiokafka's internal batching is used instead of one request/ack round trip per record.bootstrap_serversaccepts a string or list and falls back toKAFKA_BOOTSTRAP_SERVERS;KAFKA_BOOTSTRAP_SERVERSalso lands inSettings.kafka.MessageBrokerPermanentError(no retries burnt on them); connection, coordinator and rebalance errors map toMessageBrokerTransientErrorand are retried with backoff.kafkaextra (aiokafka>=0.11,<1) plusaiokafkain thetestdependency group so the unit tests import the real error and offset types.Split out of #284 as requested so that PR stays scoped to what can be tested: the base classes, PubSub and SQS/SNS.
Testing
24 unit tests (128 across the message data suite with this branch), all mock-based against
unittest.mockdoubles patched intoplugboard.library.kafka_io- no broker required. They cover offset commit contents (including out-of-order records spanning partitions), empty polls, multi-partition flattening, batched sends with exactly one flush, error mapping, settings resolution and JSON vs raw encoding.ruff,tyandtests/unitpass. No integration test against a real Kafka broker exists, which is the gap the draft status above is about; the behaviour most likely to bite is exactly the part mocks cannot show - group rebalances, session timeouts, and commit semantics under real partition assignment.Notes for whoever picks this up
feat/message-data-classes, notmain. Retarget it tomainonce feat: Message data classes #284 merges (GitHub retargets automatically if the base branch is deleted after a squash-merge, but double-check).uv.lockconflicts with feat: Message data classes #284 are likely: this branch re-addsaiokafka(and itsasync-timeoutdependency) to the lockfile that feat: Message data classes #284 removes them from. Resolving by re-runninguv lockon top of the merged base is the safest route.compose.yamlalready provides the pattern used for RabbitMQ/Redis, and the CI job that starts it exports the broker URLs - Kafka would follow the same shape (KAFKA_BOOTSTRAP_SERVERSplus atest_kafka_io.pyintegration test that round-trips a record and asserts the commit).