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

refactor: Moved external APIs out of node #2069

Merged
merged 3 commits into from
Sep 22, 2023
Merged

Conversation

SionoiS
Copy link
Contributor

@SionoiS SionoiS commented Sep 21, 2023

Description

The JSONRPC and REST APIs are no longer part of node. I created a waku_api folder for them.

This would be the first step in a bigger refactoring effort. Next step would be to move peer manager out of node as well.

Tracking #1941

@SionoiS SionoiS added the E:1.4: Sharded peer management and discovery See https://github.com/waku-org/pm/issues/67 for details label Sep 21, 2023
@SionoiS SionoiS self-assigned this Sep 21, 2023
@SionoiS SionoiS changed the title chore(refactor) Moved external APIs out of node refactor: Moved external APIs out of node Sep 21, 2023
@github-actions
Copy link

github-actions bot commented Sep 21, 2023

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2069

Built from 7f83c3d

Copy link
Collaborator

@Ivansete-status Ivansete-status left a 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 a comment: I don't see the need to move peer_manager out of node. Could elaborate a bit more on why this is interesting plz?

waku/waku_api/rest/filter/handlers.nim Outdated Show resolved Hide resolved
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.

Seems to work :)

@SionoiS
Copy link
Contributor Author

SionoiS commented Sep 22, 2023

LGTM! Thanks for it! 💯

Just a comment: I don't see the need to move peer_manager out of node. Could elaborate a bit more on why this is interesting plz?

I haven't looked into it yet but either reorganize some files only OR move the logic to the app layer.

As for why, managing peers is always app specific. A light node that only use light push and filter is not going to manage peers the same as a full node with relay enabled for multiple shards.

@SionoiS SionoiS force-pushed the chore--moving-external-api branch from 951da9e to 92bb6e3 Compare September 22, 2023 12:31
@SionoiS SionoiS merged commit 3e72e83 into master Sep 22, 2023
9 of 10 checks passed
@SionoiS SionoiS deleted the chore--moving-external-api branch September 22, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:1.4: Sharded peer management and discovery See https://github.com/waku-org/pm/issues/67 for details
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants