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

chore: Revert lightpush error handling to allow zero peer publish again succeed #2099

Merged

Conversation

NagyZoltanPeter
Copy link
Contributor

Description

Previous change on lightpush error handling caused js-waku tests failing and thus ci errors on nwaku side as well.
There had been a discussion around and decision is made to enrich the protocol for the future.
[Epic] Enhance light push protocol

Up until than we do not return error in case there is no suitable peer to publish message comes from lightpush.

Changes

  • Changed return error to debug log in lightpush callback in waku_node
  • Commented out relevant test that would fail.

Check

  • CI's js-waku should not fail

@NagyZoltanPeter NagyZoltanPeter added the bug Something isn't working label Oct 2, 2023
@NagyZoltanPeter NagyZoltanPeter self-assigned this Oct 2, 2023
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2099

Built from 002ec55

@fbarbu15
Copy link
Contributor

fbarbu15 commented Oct 2, 2023

I've looked at the js-waku tests and the problem is gone
There's still one failing test but it needs fixing on js side (missing ensureSubscriptions call)
Thanks

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@NagyZoltanPeter
Copy link
Contributor Author

I've looked at the js-waku tests and the problem is gone There's still one failing test but it needs fixing on js side (missing ensureSubscriptions call) Thanks

Hi Floring!
Great.... at least I would not worry about still failing js-waku tests.

@NagyZoltanPeter
Copy link
Contributor Author

Merging this PR to allow js-waku CI jobs succeed. Adjusted with Florin that this change fixes js-waku relevant test fail.

@NagyZoltanPeter NagyZoltanPeter merged commit f05528d into master Oct 2, 2023
9 of 10 checks passed
@NagyZoltanPeter NagyZoltanPeter deleted the chore/rewoke-lightpush-error-on-zero-publish branch October 2, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants