-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Create more robust "Event Bus" #6796
Comments
The first and non-breaking change would be to allow to define the schema of app events during startup. The registerPlugin function already has a property called export default async function register(app) {
await app.registerPlugin({
label: "Orders",
name: "orders",
version: pkg.version,
functionsByType: {
startup: [startup]
...
},
appEvents: {
schemas: eventSchemas // <-----
},
simpleSchemas: {
Order,
...
}
});
} The easiest route is to continue using simpl-schema as we can use the same simpl-schema definitions we already have for the domain objects. import { Order } from "./simpleSchemas.js";
const AfterOrderCreateSchema = new SimpleSchema({
createdBy: {
type: String
},
order: {
type: Order
}
});
export default async function register(app) {
await app.registerPlugin({
name: "orders",
...
appEvents: {
schemas: {
// Here the key is the app event name and the value is the schema
// We can provide a more detailed interface if we need to pass some extra information if the future like so:
// {
// name: "afterOrderCreate",
// schema: AfterOrderCreateSchema
// }
afterOrderCreate: AfterOrderCreateSchema
}
},
simpleSchemas: {
Order,
...
}
});
} This way we will use the same instance of the |
Regarding point 1 of the solution proposal: There is no way to list supported events at startup so we had to go through all plugins and find all the Regarding point 2 of the solution proposal: Let's say we have a custom plugin that imports pricing for products and emits an event when each product is updated. We might want to automatically publish the product with the updated prices in the catalog. This is possible by listening to the An important note here is that we want only one of the reaction instances in a multi-instance scenario to catch this event because if the event is distributed to all pods by first sending it to Redis, this will cause all pods to catch the event and trigger the publishing. From this scenario, we can conclude that when an event is emitted, only the first consumer should be able to process the event. In the case of Redis, we should configure an event to be consumed only by the first consumer. If we do that, we won't be able to consume the events from the external system by connecting to Redis directly because it might "steal" an event that is intended to be consumed by Reaction itself. That's why I think even if we switch to Redis, we should only consume the Redis events in reaction and use the webhooks plugin proposed in point 3 to allow external systems to listen to events. Unless there is a way to configure Redis to send events to only one of the reaction pods and also to any other system. |
Great comments @tedraykov Still open on what that final API subscription layer would look like. Makes sense to not make the raw PubSub available (which we can't for security reason) but I know I don't want to do something clumsy like Webhooks |
Currently we have a well-thought out and generally universally implemented "appEvent" system that allows for decoupling of components via event emitters and event listeners. This system has a few limitations that this work seeks to overcome.
So the solution would be twofold:
One of the big challenges is going to be security. How does one authorize external systems to access some events and not others? Would this be a an API key? If so how to we create and manage those.
I think this is a good first step in our plan for improve the way we move data in and out of our system while also making internal processes more robust.
Note that the first step of this work would be creating a proposal for how this would all work that would be codified as an ADR
cc/: @tedraykov @aldeed
The text was updated successfully, but these errors were encountered: