Skip to content
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

Merged
merged 25 commits into from
Jul 30, 2020
Merged

Addressable post messages. #259

merged 25 commits into from
Jul 30, 2020

Conversation

idanilt
Copy link
Contributor

@idanilt idanilt commented Jul 28, 2020

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

@idanilt idanilt changed the title New package, Addressable A lightweight standalone micro lib to create addressable post messages. Addressable post messages. Jul 28, 2020
@idanilt idanilt mentioned this pull request Jul 28, 2020
3 tasks
@idanilt idanilt requested a review from katzIdo July 28, 2020 16:46
Copy link

@stephanebenayoun stephanebenayoun left a 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/README.md Outdated Show resolved Hide resolved
packages/addressable/README.md Outdated Show resolved Hide resolved
packages/addressable/README.md Outdated Show resolved Hide resolved
packages/addressable/rollup.cjs.config.js Outdated Show resolved Hide resolved
*/
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);

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

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}

*
* @param id
*/
public remove(id: string) {

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)

Copy link
Contributor Author

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

packages/cluster-nodejs/src/Cluster/JoinCluster.ts Outdated Show resolved Hide resolved
@stephanebenayoun stephanebenayoun marked this pull request as draft July 29, 2020 09:01
@stephanebenayoun
Copy link

@idanilt CI ran for 6 hours, converting this PR to draft to avoid new run before it will be fixed.

packages/addressable/src/ConnectionClient.ts Outdated Show resolved Hide resolved
packages/addressable/tests/e2e.browser.ts Outdated Show resolved Hide resolved
packages/addressable/src/utils/eachUnique.ts Outdated Show resolved Hide resolved
packages/addressable/src/ConnectionServer.ts Outdated Show resolved Hide resolved
packages/addressable/src/ConnectionClient.ts Show resolved Hide resolved
packages/addressable/src/ConnectionClient.ts Show resolved Hide resolved
packages/addressable/src/ConnectionClient.ts Show resolved Hide resolved
packages/addressable/src/ConnectionClient.ts Show resolved Hide resolved
packages/addressable/src/ConnectionClient.ts Show resolved Hide resolved
@idanilt idanilt marked this pull request as ready for review July 29, 2020 13:44
# 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
@idanilt idanilt merged commit 6e3752e into develop Jul 30, 2020
@idanilt idanilt mentioned this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants