-
Notifications
You must be signed in to change notification settings - Fork 94
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
feat(event streaming): implement push-only streaming channels and SSE #1945
Conversation
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
77b0058
to
3e679b4
Compare
Signed-off-by: ozkanonur <work@onurozkan.dev>
3e679b4
to
04df80c
Compare
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
6cdd7b1
to
b8eca5e
Compare
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
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.
These notes are not related with this work. They don't seem right in general. Having TODOs will help us tracking them later.
@@ -1,3 +1,5 @@ | |||
// TODO: a lof of these implementations should be handled in `mm2_net` |
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: This should exclude TCP internals, as including them results in having to | ||
// handle various protocols within this function. |
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.
☝️
Signed-off-by: onur-ozkan <work@onurozkan.dev>
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.
Great Work! First review iteration!
Signed-off-by: onur-ozkan <work@onurozkan.dev>
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.
Re-approve
@KomodoPlatform/qa: Refer to this comment #1945 (comment) to see how you can test this new event streaming feature. The above comment is also a starting point for documenting this feature, if you wish to begin documentation at this early stage. If you have any questions, please direct them to @onur-ozkan |
5646e0a
to
a7a2064
Compare
…sh-only-streaming-channels
I assume this means http://127.0.0.1:7783/examples/sse/index.html? and in mm2.log |
No, it's independent HTML page(for testing SSE connection), you just need to open it in browser. You can serve using any web server like using python SimpleHTTPServer(see https://github.com/KomodoPlatform/komodo-defi-framework/blob/push-only-streaming-channels/examples/sse/README.md#listening-event-stream-from-komodo-defi-framework) |
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.
Confirm that event steam appears in browser from examples/sse/index.html, and modifying index.html to add a filter works.
Is there a plan to allow the filter to be applied via browser url in future? E.g. examples/sse/index.html?filter=BALANCES
instead of editing index.html
GUI can already achieve that by using URL parameters(e.g., https://developer.mozilla.org/en-US/docs/Web/API/Window/location) in the SSE connection string. mm2 can't know what page it's being used for. |
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.
Approved.
Event steam displayed for following places :
http://localhost:7783/event-stream
komodo-defi-framework/examples/sse/index.html
http://127.0.0.1:8000/
Filter works as expected: for ?filter=NETWORK
events are displayed, for other values of filter event stream is empty.
This implements streaming channels using mpsc(underlying part of SSE) and SSE for sending data to clients continiously. Currently, only
NETWORK
event is implemented, further events will be added after the collaboration with the GUI team.Supported platforms: all other than WASM(Simply because we can't serve a server on browser. I have a solution idea to this, but it's too early to talk about).
Enabling Event Streaming:
event_stream_configuration
property to the mm2 configuration. Use the following sample values:2- Start mm2 and wait for the initialization log message similar to:
mm2_net::network_event:54] INFO NETWORK event is activated with 0.5 seconds interval
.3- Open
examples/sse/index.html
in your browser; events should start appearing on the screen.Client-Side Filtering:
By default, clients listen to all events activated on the mm2 side. If you're interested in specific events, you can filter incoming events (using the
filter
query string parameter) to reduce network overhead.For instance, open
examples/sse/index.html
with a text editor and append?filter=TEST,DEMO,BALANCES
to theEventSource
url. After that, you will no longer receiveNETWORK
events. (Note that currently we only streamNETWORK
events on mm2 side. Filtering out theNETWORK
event will result in no events being received.)Helps to #1901