-
Notifications
You must be signed in to change notification settings - Fork 179
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
Add new websocket handler and skeleton for its deps #6630
base: master
Are you sure you want to change the base?
Add new websocket handler and skeleton for its deps #6630
Conversation
* Added websocket controller * Added mock block provider * Added data provider factory * Added websocket handler * Added websocket config * Added a tiny POC test for websocket handler
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6630 +/- ##
==========================================
- Coverage 41.23% 41.08% -0.16%
==========================================
Files 2054 2022 -32
Lines 182364 178761 -3603
==========================================
- Hits 75204 73439 -1765
+ Misses 100859 99179 -1680
+ Partials 6301 6143 -158
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Speaking of tests, I see it the following way: we should write a wrap websocket connection with our custom mock and write unit tests for websocket controller (it accepts ws conn as argument). This will require some code, so a separate task should be created for this #6631 |
* Use contexts as function arguments * Move models to folder for consistency * Change parse msg function * Simplify mock block provider to remove dedlock
* make handle_connection blocking * rename concurrent_map * use type switch instead of switch * add todos for error handling
dp.Close() | ||
c.dataProviders.Remove(id) | ||
} | ||
} |
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.
} | |
//TODO: return OK response to client | |
} |
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.
Nice! Looks good to me!
Closes #6593 #6583 #6617
Relates to #6584