Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: API to reach registered events and errors upon non-existing subscription #11

Closed
mensfeld opened this issue Feb 23, 2018 · 3 comments

Comments

@mensfeld
Copy link
Collaborator

Hey guys,

We use dry-monitor together with dry-events in karafka. In order to provide a "safe" API (safe in terms of typo mistakes etc) we've implemented two additional methods for the monitor that could be useful when working with it. Would it fit as a PR in this library?

      # Allows us to subscribe to events with a code that will be yielded upon events
      # @param event_name_or_listener [String, Object] name of the event we want to subscribe to
      #   or a listener if we decide to go with object listener
      def subscribe(event_name_or_listener)
        return super unless event_name_or_listener.is_a?(String)
        return super if available_events.include?(event_name_or_listener)
        raise Errors::UnregisteredMonitorEvent, event_name_or_listener
      end

      # @return [Array<String>] names of available events to which we can subscribe
      def available_events
        __bus__.events.keys
      end

I believe that this behavior would prevent users from subscribing to non-registered events.

Whole monitor can be found here: https://github.com/karafka/karafka/blob/master/lib/karafka/instrumentation/monitor.rb

@GustavoCaso
Copy link
Member

GustavoCaso commented Feb 25, 2018

I can help with this one 😄 or @mensfeld maybe want to open a PR?

@mensfeld
Copy link
Collaborator Author

@GustavoCaso well it shouldn't be that hard to implement as it is already there :D
@solnic since you've added the tags I guess I can implement those as described above, right?

@solnic
Copy link
Member

solnic commented Feb 26, 2018

Yes, but I reckon this should go into dry-events lib

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

No branches or pull requests

4 participants