feat(event streaming): configurable worker path, use SharedWorker#2080
feat(event streaming): configurable worker path, use SharedWorker#2080
Conversation
|
|
||
| let worker = web_sys::Worker::new("worker.js").expect("Missing worker.js"); | ||
| let worker_path = config.worker_path.as_deref().unwrap_or(DEFAULT_WORKER_PATH); | ||
| let worker = web_sys::Worker::new(worker_path).expect("Missing worker.js"); |
There was a problem hiding this comment.
Was already fixing this. Done :)
There was a problem hiding this comment.
we might have another worker.js that serves a diff purpose In the future so why not make it balance_streaming_worker.js as default ?
Also, worker.js is a very basic worker that's not doing much and we might have a more complex worker in the future
There was a problem hiding this comment.
I left it as worker.js for backward compatibility with this https://github.com/KomodoPlatform/react-komodefi-wasm/blob/streaming/worker.js ref #1978 (comment)
There was a problem hiding this comment.
I can make it balance_streaming_worker.js if agreed by @onur-ozkan and they would have to fix it in the react-komodefi-wasm branch
Also, worker.js is a very basic worker that's doing much and we might have a more complex worker in the future
Can you please elaborate on this @borngraced ?
There was a problem hiding this comment.
I believe we should get some feedback from the frontend devs. They may prefer to handle all the events from single worker file by filtering the incoming data.
It's still a single worker file for event streaming
There was a problem hiding this comment.
If they requested multiple workers in the future we can do it in another PR.
There was a problem hiding this comment.
the current worker script used for balance streaming is only logging the messages to the console..
It's just for demonstration, it doesn't have to be that way obviously.
You can handle all kinds of operations from a single worker file by filtering the incoming payload. As I mentioned, this preference should be defined by the frontend developers not by us as this affects their project structure directly.
There was a problem hiding this comment.
It's still a single worker file for event streaming
I am aware of it, I meant event streaming + different tasks on workers
There was a problem hiding this comment.
I am aware of it, I meant event streaming + different tasks on workers
In this case they can change the config I guess.
|
@smk762 @gcharang after this is merged to dev, |
…rializing `EventStreamConfiguration`
|
@smk762 @gcharang I changed the worker type to be a shared worker, so the script in // use chrome://inspect in Chrome to debug the worker
// use about:debugging in Firefox to debug the worker
self.onconnect = function(event) {
var port = event.ports[0];
console.log('A new connection to the shared worker has been successfully established.');
port.onmessage = function(e) {
console.log(e.data);
};
};Shared workers debugging can be accessed from |
borngraced
left a comment
There was a problem hiding this comment.
according to this doc, and usage of shared worker..do you think we need it here just stick to dedicated worker without much stuffs going on in this code.
it was requested by @naezith since normal worker doesn't fit his needs in the flutter codebase. Plus, I think it's good to have event streaming worker shared and accessed from multiple contexts. |
* dev: feat(indexeddb): advanced cursor filtering impl (GLEECBTC#2066) update dockerhub destination repository (GLEECBTC#2082) feat(event streaming): configurable worker path, use SharedWorker (GLEECBTC#2080) fix(hd_tests): fix test_hd_utxo_tx_history unit test (GLEECBTC#2078) feat(network): improve efficiency of known peers handling (GLEECBTC#2074) feat(nft): enable eth with non fungible tokens (GLEECBTC#2049) feat(ETH transport & heartbeats): various enhancements/features (GLEECBTC#2058)
This PR allows any worker path to be used instead of hardcoded
worker.jsfile. The worker path can be specified usingworker_pathinevent_stream_configuration, if not specifiedworker.jswill be used as default. Example: