-
Notifications
You must be signed in to change notification settings - Fork 56
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: metadata protocol shard subscription #2149
Conversation
You can find the image built from this PR at
Built from accfeb7 |
a1614cc
to
b0c3c2c
Compare
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.
really good, thanks for this.
- minor comment regarding exception raising
- can we add some units tests for this?
For the subscription part? Sure! |
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.
lgtm!
b9f2f14
to
994b9d4
Compare
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.
LGTM! Thanks for it!
Just added a few nitpicks but not blocking ones :)
On the other hand, I realized that the metadata protocol is being mounted within the creation of the Waku node. Why is that needed @alrevuelta? I wonder if we could follow the standard approach that we are following in other protocols so that all is more consistent.
0501ad1
to
cb1d31b
Compare
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.
Thanks! Good step forward.
cb1d31b
to
31ea8e3
Compare
31ea8e3
to
c19b92a
Compare
Description
The shards a node support can dynamically changes and the metadata protocol must be updated at run-time to reflect this.
Changes
Tracking #1940
Fix #2105