-
Notifications
You must be signed in to change notification settings - Fork 11
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
Addressable post messages. #259
Conversation
… addressable post messages.
# Conflicts: # yarn.lock
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.
Partial review
packages/addressable/src/Node.ts
Outdated
*/ | ||
public subscribe(fn: (peer: Peer) => void): () => void { | ||
// Peers act like unique reply subject | ||
return map((o: Peer) => ({ id: o.key, port: o.value }), eachUnique(this.peers$)).subscribe(fn); |
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.
Will not emit anything when a peer is removed.
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.
I didn't handle it yet
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.
I fix it, it will emit {id, undefined}
packages/addressable/src/Node.ts
Outdated
* | ||
* @param id | ||
*/ | ||
public remove(id: string) { |
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.
No one is using this (and it's not covered by any use case defined in tests)
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.
I didn't handle it yet
@idanilt CI ran for 6 hours, converting this PR to draft to avoid new run before it will be fixed. |
# Conflicts: # packages/browser/package.json # packages/cluster-browser/package.json # packages/cluster-nodejs/package.json # packages/node/package.json # packages/routers/package.json # packages/rsocket-adapter/package.json # packages/rsocket-ws-gateway/package.json # packages/scalecube-discovery/package.json # packages/scalecube-microservice/package.json # packages/transport-browser/package.json # packages/transport-nodejs/package.json # packages/utils/package.json
In order to use scalecube in the browser we need infrastructure to send and receive messages
At this moment we don't have it as result our cluster and transport implementation is much more complex than it should be
Beside that we need extra utils and it's not working for all the use cases
Addressable package will hide the complexity and allow scalecube to work very similar to server implementation and utilize the addresses
#256