-
Couldn't load subscription status.
- Fork 49
feat: create @waku/run package for local dev env #2678
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
Conversation
size-limit report 📦
|
|
add new package to |
9ead3eb to
9cab60e
Compare
4a345c3 to
6bc711d
Compare
|
you must update https://github.com/waku-org/js-waku/blob/master/.release-please-manifest.json too with adding initial version from which release will start |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
packages/run/cspell.json
Outdated
| @@ -0,0 +1,27 @@ | |||
| { | |||
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.
this must be used from the root of the repo
packages/run/.gitignore
Outdated
| @@ -0,0 +1,11 @@ | |||
| # Environment variables | |||
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.
this must be used from the root of the repo too
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.
this file is still present
| @@ -0,0 +1,142 @@ | |||
| # Environment variable definitions | |||
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.
can we actually run nwaku-compose with needed configs for us here?
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.
nwaku-compose comes with a bunch of other stuff we don't need (e.g. grafana/prometheus)
packages/run/package.json
Outdated
| "homepage": "https://github.com/waku-org/js-waku/tree/master/packages/run#readme", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/waku-org/js-waku.git" |
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.
nit: git prefix to use for url #2688
packages/run/package.json
Outdated
| "check:spelling": "cspell \"{README.md,src/**/*.ts,scripts/**/*.ts,tests/**/*.ts}\"" | ||
| }, | ||
| "engines": { | ||
| "node": ">=18" |
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.
we use 24 now #2688
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.
actually we have 22, CI uses 24
packages/run/package.json
Outdated
| "mocha": "^10.3.0", | ||
| "npm-run-all": "^4.1.5", | ||
| "ts-node": "^10.9.2", | ||
| "tsx": "^4.7.0", |
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.
why do you need tsx?
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.
it was used to run scripts in package.json, but can be safely removed and use dist/... instead
packages/run/package.json
Outdated
| "chai": "^4.3.10", | ||
| "cspell": "^8.6.1", | ||
| "esbuild": "^0.21.5", | ||
| "express": "^4.21.2", |
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.
express seems completely unnecessary
as I see it is used to check for REST endpoints
we should not do such testing and just check in our CI that docker is getting run with correct commands - smoke testing
anything else does not add value to our CI
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.
I was using this for testing against the env via browser, but those tests (and this dependency) can be removed. i think the node tests are sufficient (and are not run in CI anyways)
packages/run/scripts/start.ts
Outdated
| }; | ||
|
|
||
| function checkAndPullImages(): void { | ||
| const nwakuImage = process.env.NWAKU_IMAGE || "wakuorg/nwaku:v0.36.0"; |
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.
all the configs and default values must be in const or config file
this includes: nwaku image, postgres image, CLUSTER_ID below
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.
in other files too
please, double check it as I see that there are still some magic numbers
|
|
||
| # Shared nwaku configuration | ||
| x-nwaku-base: &nwaku-base | ||
| image: ${NWAKU_IMAGE:-wakuorg/nwaku:v0.36.0} |
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.
it looks like you don't need to hardcode here any versions of nwaku, postgres, or TC PORT - seems everything get's passed from our script - so it is safe to assume the values are present and no fallback is needed
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.
left comments, overall looks super useful!
bdfdcf2 to
ff730db
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
Problem / Description
The most accurate testing of js-waku applications requires running against an nwaku fleet.
These are not always readily accessible, and depend on external connectivity, which can be especially spotty during hackathons.
Solution
Provide a new package
@waku/runthat a dev can use via npx to quickly spin up two nwaku nodes pre-configured to connect with each other.Command prints minimum light node config necessary for dev to create a js-waku node that connects to the local fleet.
Notes
Checklist