Skip to content

Conversation

@awrichar
Copy link
Contributor

@awrichar awrichar commented Jun 15, 2022

Part of FIR-12

The event plugins should be singletons, but subscription manager is initialized once per namespace. A single websocket connection may facilitate listeners to multiple subscriptions (and therefore multiple namespaces), and the events need to be delivered to the correct subscription manager.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar requested a review from shorsher June 15, 2022 15:19
@awrichar awrichar marked this pull request as draft June 15, 2022 15:19
awrichar added 3 commits June 20, 2022 15:00
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar marked this pull request as ready for review June 20, 2022 22:16
Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename of listener to handler for the callbacks makes sense.

Moving towards a situation where plugins have a separate handler registered for each namespace, and they are responsible for maintaining a map makes sense.

Thanks for on validating my understanding that this PR is one step on the journey (for the Events plugin) to a world where the following architecture is true for all plugins:

  • plugins are singletons and may serve many namespaces
  • orchestrators and managers are one-per-namespace
  • callback handlers are one-per-namespace, so when they are registered ideally the plugin can be told exactly one namespace that they will service, so it delivers only those events

Future work will address this on DX/Shared storage etc. (once the kinks are worked out in that on the connector side for DX in particular)

@peterbroadhurst peterbroadhurst merged commit 7a340a9 into hyperledger:main Jun 21, 2022
@peterbroadhurst peterbroadhurst deleted the events branch June 21, 2022 14:17
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.

2 participants