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

feat: add support for cors and integrate waku-frontend #30

Closed
wants to merge 3 commits into from

Conversation

weboko
Copy link

@weboko weboko commented Dec 4, 2023

No description provided.

@weboko
Copy link
Author

weboko commented Dec 4, 2023

@gabrielmer do we need to use custom nwaku image or latest to have the fix for cors?

@@ -86,6 +86,7 @@ exec /usr/bin/wakunode\
--rest=true\
--rest-address=0.0.0.0\
--rest-port=8645\
--rest-allow-origin:"localhost:*"\
Copy link
Collaborator

Choose a reason for hiding this comment

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

mm, is this flag an nwaku flag? can't find it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This flag is going to be introduced and is not yet merged to master. We need to first merge a PR in nim-presto in order to create the PR in nwaku that would introduce this flag.

I built a custom image that includes the changes in nwaku and in nim-presto so we can use in the meantime, and in that image that flag is defined.

So if we want to use that flag, we need to also modify the image used by nwaku.

Copy link
Collaborator

@alrevuelta alrevuelta 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 the PR. left a comment.

@gabrielmer
Copy link
Contributor

gabrielmer commented Dec 5, 2023

@gabrielmer do we need to use custom nwaku image or latest to have the fix for cors?

To have the fix for CORS we currently need the custom image wakuorg/nwaku:debug-cors-wa until fixes get merged to both nim-presto and nwaku.

If necessary I can create an image with a more meaningful/appropriate name

@weboko
Copy link
Author

weboko commented Dec 5, 2023

This PR should be ready now.

Copy link
Contributor

@gabrielmer gabrielmer left a comment

Choose a reason for hiding this comment

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

LGTM until we get the CORS fix into master. @alrevuelta what do you think?

# TODO: migrate to waku-org
image: docker.io/alrevuelta/waku-frontend:latest
ports:
- 127.0.0.1:8080:80
Copy link
Collaborator

Choose a reason for hiding this comment

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

if the frontend listens on 3000, shouldnt this be
127.0.0.1:8080:3000

Accepting connections at http://localhost:3000

Copy link
Author

Choose a reason for hiding this comment

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

It listens 8080

@alrevuelta
Copy link
Collaborator

LGTM until we get the CORS fix into master. @alrevuelta what do you think?

I would prefer to wait until we have the cors issue fixed in a release, unless this blocks something else.

@weboko
Copy link
Author

weboko commented Dec 6, 2023

I would prefer to wait until we have the cors issue fixed in a release, unless this blocks something else.

Fine by me, just to clarify - Chat is not working without it

@Ivansete-status
Copy link
Contributor

Morning @weboko ! I think this can be closed now given that we currently supporting the frontend

@Ivansete-status
Copy link
Contributor

@weboko confirmed that we can close that one. Thanks for it 🙌 !

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.

4 participants