feat: switches to REST for calling nwaku messages/subscription endpoints #1852
+99
−34
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adklempner
force-pushed
the
feat/messages-subscription-rest
branch
from
dee052c
to
6bde07c
Compare
size-limit report 📦
|
danisharora099
approved these changes
Feb 15, 2024
| @@ -255,7 +283,7 @@ export class ServiceNode { | |||
| ]); | |||
| } | |||
|
|
|||
| async messages( | |||
| async messagesRpc( | |||
There was a problem hiding this comment.
nit: do we need to update the new function name within any tests?
There was a problem hiding this comment.
do we even need to keep it?
weboko
reviewed
Feb 15, 2024
| throw error; | ||
| } | ||
| }, | ||
| { retries: 5 } |
There was a problem hiding this comment.
just wondering why 5 retries?
There was a problem hiding this comment.
copied from old function. removed and it doesn't break tests
weboko
approved these changes
Feb 15, 2024
adklempner
force-pushed
the
feat/messages-subscription-rest
branch
2 times, most recently
from
3547160
to
6cd419b
Compare
This commit modifies functions in ServiceNode to use the REST API instead of JSON RPC when reading messages for a pubsub topic or content topic, and when ensuring a nwaku node is subscribed to a pubsub topic. Also modifies default Docker params to enable the rest API and provide a port.
adklempner
force-pushed
the
feat/messages-subscription-rest
branch
from
6cd419b
to
739f2bc
Compare
alrevuelta
reviewed
Feb 29, 2024
| @@ -109,6 +109,7 @@ export default class Dockerode { | |||
| HostConfig: { | |||
| AutoRemove: true, | |||
| PortBindings: { | |||
| [`${restPort}/tcp`]: [{ HostPort: restPort.toString() }], | |||
| [`${rpcPort}/tcp`]: [{ HostPort: rpcPort.toString() }], | |||
There was a problem hiding this comment.
unsure how this is used but rpc wont longer exist. eg rpcPort.toString().
| @@ -49,6 +50,7 @@ export class ServiceNode { | |||
| private websocketPort?: number; | |||
| private readonly logPath: string; | |||
| private rpcPort?: number; | |||
| if (err) reject(err); | ||
| resolve(ports); | ||
| resolve({ | ||
| rpcPort: ports[0], |
| @@ -138,13 +148,15 @@ export class ServiceNode { | |||
| Object.assign( | |||
| mergedArgs, | |||
| { | |||
| rest: true, | |||
| restPort, | |||
| rpcPort, | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
nwaku will be deprecating JSON RPC API. We will no longer be able to use those endpoints in e2e tests and must switch to using the REST API.
Solution
This PR migrates two endpoints used in e2e from RPC to REST:
GET /relay/v1/messages/andPOST /relay/v1/subscriptionsAlso updates utils for running the Docker image to provide arguments to enable
restand run it onrestPortNotes