-
Notifications
You must be signed in to change notification settings - Fork 54
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(rln-relay): removed rln from experimental 🚀 #2001
Conversation
528d66a
to
6480141
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.
had to be deleted - actions was acting out
@vpavlin is there a way to leave the file in but disable the workflow?
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.
Yes, we can manually disable workflows through GH actions UI
You can find the image built from this PR at
Built from a4839d1 |
9820019
to
c941131
Compare
c941131
to
429389e
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! left a comment.
if node.wakuRlnRelay == nil: | ||
return false | ||
return await node.wakuRlnRelay.isReady() | ||
if node.wakuRlnRelay == nil: |
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.
mm with this a node that doesnt have rln enabled will never be ready. perhaps we should add the return true
back?
but yeah, problem is, if wakuRlnRelay=nil
is it because i) rln is not enabled and we are ready or because ii) rln is enabled but not mounted and in this case we are not ready.
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.
wakuRlnRelay is nil whenever the --rln-relay
flag is not passed in
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.
Addressed in 88bfe6e
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 apart from the @alrevuelta's comment on isReady:) But I don't see any "right and quick" solution
@NagyZoltanPeter would you maybe know why the ci failed on this test?
|
Ah .... Actually I should have foresee this. For now I would suggest remove failure case from test_rest_health to quick fix it for now. I will add a new issue to track this further. |
d3f6d5e
to
d93bf61
Compare
Description
This PR removes rln from the experimental feature set, as the acceptance criteria for #1906
Changes
when defined(rln)
note: i left in theEXPERIMENTAL
flag for any future protocol that will be experimental, it seems tomake sense to leave it in instead of removing it and then adding it back in when its needed, but open to
opinions
Removed the
EXPERIMENTAL
flag, and all its usagesHow to test
make -j12 wakunode2
make -j12 test
Issue
closes #1906