Conversation
77282de to
e97bf59
Compare
hanami-utils.gemspec
Outdated
| spec.required_ruby_version = '>= 2.3.0' | ||
|
|
||
| spec.add_dependency 'transproc', '~> 1.0' | ||
| spec.add_dependency 'wisper', '~> 2.0.0' |
There was a problem hiding this comment.
just a question: could we drop wisper dependency? Or make it not require dependency for hanami-utils?
There was a problem hiding this comment.
That's a good question. I don't like the idea of adding more dependencies than needed, but idea of using Wisper was described here: https://discourse.hanamirb.org/t/hanami-2-0-ideas/306/9
That's why I thought that it's something we want to use for this case.
There was a problem hiding this comment.
I think what @davydovanton was saying is that we can make wisper an optional dependency for Hanami::Utils, by only requiring it for Hanami::Events, not all of Hanami::Utils. Doing something like:
if defined?(Wisper)
require 'hanami/events/backends/wisper' # or something...
else
raise "You must require `wisper` in your Gemfile in order to use `Hanami::Events`"
end
This would also let us add different backends later, which could be useful.
There was a problem hiding this comment.
Thanks. I like this idea. So this way we can get rid from hard-coded dependency and let clients decide which backend they want to use.
|
@davydovanton thanks for the feedback, I mentioned #160 PR in my description to current one. My intent was to implement the same API for |
|
@davydovanton @cllns @jodosha I've updated PR:
Let me know what you think. |
|
@smakagon you can add wisper as a development dependency :) |
|
Done. |
jodosha
left a comment
There was a problem hiding this comment.
@smakagon Thanks for this PR.
We're missing an "isolation" spec to make sure that the absence of wisper will raise an error. For this purpose, please have a look at: https://github.com/hanami/utils/blob/master/spec/isolation/hanami/utils/json/json_spec.rb
Hint: you should move wisper from dev deps in gemspec to a separated Gemfile group named :events. That will allow to selectively load that group via Bundler.
What we should avoid, once again, is: ..global state! 🎉
This design uses class methods which registers globally the subscriptions.
What we should aim to is:
events = Hanami::Events.new
events.subscribe(:signup, mailer)
events.broadcast(:signup, user: 1)| # | ||
| # @since 0.1.0 | ||
| module Hanami | ||
| require 'hanami/events' |
| begin | ||
| require 'hanami/events/backends/wisper' | ||
| rescue LoadError | ||
| raise "Couldn't find `wisper` gem. Please, add `gem 'wisper', '2.0.0'` to Gemfile in order to use `Hanami::Events`" |
There was a problem hiding this comment.
Maybe we should do ~> 2.0, assuming wisper uses SemVer? rather than locking them down to one very specific version
|
Hello, thanks for PR! After discussion with @jodosha, we decided that this library should look like another gem. Why different adapters?I think hanami events should be a library for building Event-Driven Architecture. In this case, a library should work with different hanami project instances: In this case, we can provide different transport layers for a event storage like:
And that's why we need to replace wispers gem as an adapter. From which hanami events should be?I think that we need to split all logic to 3 different parts:
All these parts should be without a global state. And here my ideas about API for all this. Broadcasterevents = Hanami::Events.new(:adapter)
events.broadcast('user.created', user: user)Subscriberkafka_events = Hanami::Events.new(:kafka)
memory_events = Hanami::Events.new(:memory)
memory_events.subscribe('user.signup', Mailer)
# or in class
class Signup
inclide Hanami::Events::Subscriber
subscribe_to memory_events, 'user.signup'
subscribe_to kafka_events, 'user.signup'
def call(payload)
# ...
end
endAlso, I think we should have an ability for subscribing to all events: kafka_events = Hanami::Events.new(:kafka)
kafka_events.listen do |event|
event.name # => 'user.signup'
event.payload # => { ... }
endRoutingRouting should route all events and call specific code for each one. It will be helpful for manage all events in one place (look like HTTP routers): class WebHandler < Hanami::Events::Handler
on('user.created') { |payload| UserRepository.new.create(payload) }
on('user.updated') { |payload| payload }
end
class AnaliticHandler < Hanami::Events::Handler
on('*', AnaliticSender)
on('user.*') { |payload| payload }
on('user.signup') { |payload| payload }
end
class NotificationHandler < Hanami::Events::Handler
on('user.created') { |payload| payload }
on('post.created') { |payload| payload }
end
class EventRouting < Hanami::Events::Routing
mount UserHandler, Hanami.app?(:user_events) # load specific handlers for instance
mount AnaliticHandler
mount NotificationHandler
end
events = Hanami::Events.new(:adapter)
events.listen { |event| EventRouter.new.resolve(event) }I think we can try to implement proof of concept for this gem. WDYT? |
|
@davydovanton @jodosha looks really good. Thanks for a detailed description of idea. Let's implement it as a separate gem with support for different adapters. I'm in :) |
|
@davydovanton Thanks for the detailed explanation. However, I'm not convinced with a few points above:
Can we think of a simplified proof of concept, without these features? It's easier to incrementally build on top of a set of core features. We can think to start with memory and redis adapters. They are easy to reason about, to develop, and deploy. @davydovanton Can you update the Hanami 2.0 ideas thread with your proposal? https://discourse.hanamirb.org/t/hanami-2-0-ideas/306/47 Thanks! |
|
I'm closing this in favor of https://github.com/hanami/events |

Implementation of
Hanami::Eventsusing Wisper gem as a backend.Implements the API provided in this PR: #160
Usage
Subscribe With A Proc
Subscribe With An object
Include Mixin