Adds Kaffe.MessageHandler behaviour #154
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Kaffe.MessageHandler behaviour module to centralize documentation and provide better type specs for Kafka message handling. The change improves developer experience by consolidating scattered documentation and enabling better compiler warnings for library consumers.
- Adds
Kaffe.MessageHandlerbehaviour with centralized documentation and type specs - Updates documentation to reference the new behaviour instead of inline explanations
- Reorganizes documentation in producer module by moving inline docs to function definitions
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| mix.exs | Bumps version to 1.28.0 and adds CHANGELOG.md to documentation |
| lib/kaffe/message_handler.ex | New behaviour module defining handle_messages/1 callback with comprehensive documentation |
| lib/kaffe/consumer_group/worker/worker.ex | Updates worker documentation to reference new MessageHandler behaviour |
| lib/kaffe/consumer.ex | Adds message() type definition for better type safety |
| lib/kaffe/producer.ex | Refactors documentation by moving inline docs to function definitions |
| README.md | Updates usage examples to show MessageHandler behaviour implementation |
| CHANGELOG.md | Documents the new feature in version 1.28.0 |
TylerWitt
force-pushed
the
handler-behaviour
branch
from
68d7e4c to
f8b3c4a
Compare
|
I think this can get lumped in to 1.28.0 pretty easy |
|
|
||
| ```elixir | ||
| def application do | ||
| [applications: [:logger, :kaffe]] |
There was a problem hiding this comment.
Maybe a dumb question, but is this no longer needed?
There was a problem hiding this comment.
Yeah, this is super outdated.
Elixir as 1.4 infers which applications to include based on your deps. See https://elixir-lang.org/blog/2017/01/05/elixir-v1-4-0-released/
There's now the extra_applications tag, which is only needed when you use something that is not in the deps list (typically things from erlang, like :ssl or :logger).
| `handle_messages/1` This function (note the pluralization) will be called with a *list of messages*, with each message as a map. Each message map will include the topic and partition in addition to the normal Kafka message metadata. | ||
| 1. Define a `handle_messages/1` function in the provided module implementing the `Kaffe.MessageHandler` behaviour. | ||
|
|
||
| The module's `handle_messages/1` function _must_ return `:ok` or Kaffe will throw an error. The Kaffe consumer will block until your `handle_messages/1` function returns `:ok`. |
There was a problem hiding this comment.
Would it be helpful to have this kind of comment somewhere else?
There was a problem hiding this comment.
I think the README is a good place for it, unless we want to make a separate Getting Started.md or similar. Thoughts?
There was a problem hiding this comment.
I think the README is a fine place for it. A Getting Started.md file might be a good idea, but can probably be a separate PR if we do it
This fixes some hex formatting, removes outdated documentation, and fixes what I think is the last of the duplicated `@doc` warnings
This should make implementation easier, and also makes the contract a little more clear. It will also give better warnings in implementing code if (as an example) the wrong return type is possible.
TylerWitt
force-pushed
the
handler-behaviour
branch
from
f8b3c4a to
214719f
Compare
fatcatt316
left a comment
fatcatt316
left a comment
There was a problem hiding this comment.
One tiny typo with it's, but besides that, this looks good to me! 👍
| `handle_messages/1` This function (note the pluralization) will be called with a *list of messages*, with each message as a map. Each message map will include the topic and partition in addition to the normal Kafka message metadata. | ||
| 1. Define a `handle_messages/1` function in the provided module implementing the `Kaffe.MessageHandler` behaviour. | ||
|
|
||
| The module's `handle_messages/1` function _must_ return `:ok` or Kaffe will throw an error. The Kaffe consumer will block until your `handle_messages/1` function returns `:ok`. |
There was a problem hiding this comment.
I think the README is a fine place for it. A Getting Started.md file might be a good idea, but can probably be a separate PR if we do it
There are a lot of places in the docs where we were explaining the return typing and input for the
handle_messages/1callback from kafka consumption.This behaviour module
@specso library consumers get better warnings