Passing state to handle_setup
#67
Conversation
- If the Consumer defines handle_setup/1 (instead of /2) then it will
receive the entire current state and can update it by returning `{:ok,
state}`.
- Make `queue` an optional argument to avoid providing an empty value if
RabbitMQ is expected to generate the queue name in `handle_setup/1`.
- Add test coverage for a Consumer without `handle_setup`.
- Pass the full state if `handle_setup/2` is provided as a callback. - Allow the result of `handle_setup` to modify the state. - Add `setup_opts` as an init argument to store in the state.
|
Thanks @Odaeus! I will try and review this today. |
The queue name might be set dynamically in `handle_setup` instead of supplying the value on start. If there is no queue name by the time we start consuming then stop the server.
Odaeus
force-pushed
the
handle_setup_state
branch
from
3341fb9 to
26c3d69
Compare
|
@nsweeting I've pulled out the "optional queue opt" to a separate commit and added something to return a clear error if there's no queue name by the time it reaches |
|
Hey @nsweeting, just wanted to let you know that I'm planning to merge this and related changes into the master branch of my fork this week. Still happy to make any changes when requested, with the aim that we will eventually be using your official version. |
There was a problem hiding this comment.
Since we are still a pre-1.0 release - I would be fine with making this a "breaking" change in order to maintain this as a cleaner implementation. As long as we make sure to note the change in the release notes.
Based on the above - I think we can drop support for handle_setup/2 and only deal with handle_setup/1 with the state being passed. The expected return value should be {:ok, state} .
I don' t think we need to support a :skip return value. Unless you have a requirement I don't see right now?
This backwards-incompatible change simplifies the `Consumer.handle_setup` usage and implementation.
This backwards-incompatible change simplifies the usage and implementation of `Producer.handle_setup` while keeping the flexibility of providing the entire state rather than just the channel. Also makes it consistent with the equivalent function in `Consumer`.
|
Sorry for the delay on this! I've removed Hopefully I've updated all the documentation in the right places. |
Looks great @Odaeus! Will merge and cut a new release. Thanks. |
|
@Odaeus Had to submit PR #73 to support |
|
Thanks so much! |
Resolves issue #64.
Please review and let me know if the code fits to how you want it to work!