-
Notifications
You must be signed in to change notification settings - Fork 10
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
chore(ws): simplify messaging server #424
chore(ws): simplify messaging server #424
Conversation
/build_test |
Workflow started at 4/29/2024, 9:40:46 AM. View Actions Run. |
No GraphQL schema changes detected. |
No OpenAPI schema changes detected. |
CI build and push: All tests pass ✅ (JDK17) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Just a rebasing.
26757bb
to
6f57034
Compare
/build_test |
Workflow started at 4/30/2024, 11:40:23 AM. View Actions Run. |
No OpenAPI schema changes detected. |
No GraphQL schema changes detected. |
CI build and push: All tests pass ✅ (JDK17) |
Welcome to Cryostat3! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/main
Related to #423
Description of the change:
Simplifies the MessagingServer (WebSocket notifications) implementation to remove the explicit thread synchronization and message ordering.
Motivation for the change:
Using the EventBus is simpler and less error-prone than manually using synchronization features like blocking queues. Error handling is also improved in this PR, so if sending a notification to any connected client fails then this failure will be logged.
How to manually test: