Refactor notification stream syncing code for tandem use by bot service #2280
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
clemire
force-pushed
the
crystal/refactor-event-processing
branch
from
62e06e4 to
2477d89
Compare
bas-vk
reviewed
Feb 7, 2025
|
One thing to note is that currently the notification service misses events during downtime. There is still a wish to make the sync cookies persistent so the notification service can resume processing events after it resumed working. This is probably also required for other services. |
bas-vk
approved these changes
Feb 7, 2025
Oof, we probably don't want to miss events for the bot registry service. |
clemire
force-pushed
the
crystal/refactor-event-processing
branch
from
d4550ef to
47a2e3c
Compare
…as well, because initially the bot service will also track all channel streams as well as user and user inbox streams.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The actual streams tracker will be sufficiently different for each service that it makes sense to go ahead and implement them separately: the notification service cares about all DM, GDM, and channel streams, and tracks all of these, but the bot registry service only cares about streams associated with a registered bot, and the channels in the spaces that bot belongs to. Furthermore, channel streams are sometimes picked up on allocation by the notification service, but they are almost always going to be added retroactively for the bot registry service at the point at which we detect a bot has been added to a channel.
However, the logic for keeping an up-to-date internal copy of the view of each stream, and for syncing that stream, are very similar. This PR refactors those common elements into a shared library and plugs them back into the notification service.