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

Support for custom event publishing strategy #8

Merged
merged 1 commit into from
Nov 1, 2016

Conversation

prgTW
Copy link

@prgTW prgTW commented Oct 15, 2016

There are 2 strategies of handling events: always and predefined. I've added ability to define custom one (using a backward compatible configuration).

simple_bus_asynchronous:
    events:
        strategy:
            strategy_service_id: custom_strategy_service

TODO

  • Add tests
  • Add docs

Related

Related to: #6

@prgTW prgTW force-pushed the feat/custom-event-strategy branch 2 times, most recently from 8c83bf4 to 868cd0f Compare October 15, 2016 14:55
@coveralls
Copy link

coveralls commented Oct 15, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 868cd0f on prgTW:feat/custom-event-strategy into 41cb103 on SimpleBus:master.

@prgTW
Copy link
Author

prgTW commented Oct 18, 2016

@matthiasnoback what do You think?

@cmodijk
Copy link
Member

cmodijk commented Oct 18, 2016

Hi @prgTW I'm the new maintainer of the SimpleBus repository's.

The unhandled only works if the UndefinedCallable exception is thrown and if i'm correct this is not the case for events. This is why have the predefined method because that one looks at al the events registert as async and only sends the messages to the queue if there is one registered as such. But it will always still send it to the local event listeners.

The idea behind this is that you can have multiple event listeners on the same event and this is the reason that we have the always method in this case all events are always handled on the local version and also send in async version to allow a async listener to listen to this. But if there are no listeners at al that is also fine in the case of events just get discarded.

We could allow the change you did to make the service configurable but the unhandled version will not work (if i'm correct).

@prgTW prgTW force-pushed the feat/custom-event-strategy branch from 868cd0f to da995ef Compare October 20, 2016 07:11
@prgTW prgTW changed the title additional "unhandled" event strategy + custom strategy support Support for custom event publishing strategy Oct 20, 2016
@coveralls
Copy link

coveralls commented Oct 20, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling da995ef on prgTW:feat/custom-event-strategy into 41cb103 on SimpleBus:master.

@prgTW
Copy link
Author

prgTW commented Oct 20, 2016

@cmodijk thanks for Your reply.

You were absolutely correct of that the 'unhandled' strategy will not work so I followed Your comment and left only the ability for custom strategy support - after all it's enough for customizations.

I've updated the code, pull request title and description. I hope everything is ok now :)

@cmodijk
Copy link
Member

cmodijk commented Oct 20, 2016

@prgTW Yes, thanks for this. One question i have personally what method are going to implement if this get's merged?

ps; I will get back to you in a few days to review it and merge it.

@prgTW
Copy link
Author

prgTW commented Oct 20, 2016

@cmodijk I'm going to implement a complete opposite strategy to predefined which will publish event only if it has no synchronous subscriber but (unlike predefined) won't require an asynchronous subscriber to be registered.

In my system I have something like internal and external events. External events are for 3rd party systems and therefore should use always strategy, but internal events are used for async processing within the application - we should use predefined here. In combination I wanted to create a custom strategy which will always push an instance of ExternalEventInterface but publish InternalEventInterface events based on them having an async subscriber

@cmodijk cmodijk merged commit ad7d87f into SimpleBus:master Nov 1, 2016
@cmodijk
Copy link
Member

cmodijk commented Nov 1, 2016

Merged! I'm doing some small stuff tonight i hope to create a release later this evening.

@prgTW prgTW deleted the feat/custom-event-strategy branch November 2, 2016 13:52
@prgTW
Copy link
Author

prgTW commented Nov 2, 2016

looking forward then, thanks :)

@cmodijk
Copy link
Member

cmodijk commented Nov 2, 2016

@prgTW Done, release 2.2.0 is out

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

Successfully merging this pull request may close these issues.

3 participants