Conversation
🦋 Changeset detectedLatest commit: c74d431 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughReorganized Docker Compose infrastructure from a single root Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR reorganizes the Docker setup into a
Confidence Score: 4/5Not safe to merge as-is — the orchestrator compose file is missing postgres env vars, which will cause CI integration tests to fail. One P1 defect: the postgres service in docker-compose.orchestrator.yml has no env_file, so POSTGRES_PASSWORD is unset and the container fails to start, breaking orchestrator.ts-based CI tests. The fix is a small addition. All other changes are well-structured and manually verified. docker/docker-compose.orchestrator.yml — postgres service missing env_file block Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph docker/
DC[docker-compose.yml\nmainnet stack]
DCDEV[docker-compose.devnet.yml\ndevnet stack]
DCORCH[docker-compose.orchestrator.yml\nCI / orchestrator.ts]
ENV[.env.docker-compose\nshared env defaults]
end
subgraph docker/services/
SVC_EI[ensindexer.yml]
SVC_EA[ensapi.yml]
SVC_ER[ensrainbow.yml]
SVC_ADMIN[ensadmin.yml]
SVC_PG[postgres.yml]
SVC_DV[devnet.yml]
end
DC -->|extends| SVC_EI
DC -->|extends| SVC_EA
DC -->|extends| SVC_ER
DC -->|extends| SVC_ADMIN
DC -->|extends| SVC_PG
DC -->|env_file| ENV
DCDEV -->|extends| SVC_EI
DCDEV -->|extends| SVC_EA
DCDEV -->|extends| SVC_ER
DCDEV -->|extends| SVC_ADMIN
DCDEV -->|extends| SVC_PG
DCDEV -->|extends| SVC_DV
DCDEV -->|env_file| ENV
DCORCH -->|extends| SVC_DV
DCORCH -->|extends| SVC_PG
DCORCH -. "missing env_file" .-> ENV
ORC[orchestrator.ts] -->|DockerComposeEnvironment| DCORCH
Reviews (1): Last reviewed commit: "add docker directory with several docker..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/integration-test-env/src/orchestrator.ts (1)
14-17:⚠️ Potential issue | 🟡 MinorStale design-decision comment.
The header comment still describes Phase 1 as starting Postgres+devnet "from the root docker-compose.yml". With this change the orchestrator now uses
docker/docker-compose.orchestrator.yml. Please update the comment to reflect the new location so the file-level documentation stays accurate.📝 Proposed tweak
- * - Postgres and devnet are started from the root docker-compose.yml via - * testcontainers DockerComposeEnvironment, ensuring the orchestrator always - * uses the same images and configuration defined there. + * - Postgres and devnet are started from docker/docker-compose.orchestrator.yml + * via testcontainers DockerComposeEnvironment, ensuring the orchestrator + * always uses the same images and configuration defined there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/integration-test-env/src/orchestrator.ts` around lines 14 - 17, Update the stale header comment in the file-level doc (the "Design decisions" block in orchestrator.ts) to reflect that Phase 1 now starts Postgres and devnet using docker/docker-compose.orchestrator.yml (not the root docker-compose.yml); edit the sentence that references "root docker-compose.yml" to mention "docker/docker-compose.orchestrator.yml" and ensure the comment still conveys that Testcontainers' DockerComposeEnvironment is used so the orchestrator uses the images/config from that compose file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensindexer/.env.local.example`:
- Around line 246-247: Update the inline comment above LABEL_SET_ID to match the
project's casing and punctuation conventions by changing it to refer to
ENSRainbow's LABEL_SET_ID and ending the sentence with a period (i.e., use
"ENSRainbow's LABEL_SET_ID."). Ensure the symbol LABEL_SET_ID remains unchanged
and only the comment text is adjusted.
In `@docker/docker-compose.devnet.yml`:
- Around line 69-75: The devnet compose declares volumes named postgres_data and
ensrainbow_data that collide with base/mainnet stacks — rename these volumes to
stack-specific names (e.g., postgres_data_devnet, ensrainbow_data_devnet) and
update all volume references in the devnet compose services to use the new names
(ensure services referencing postgres_data and ensrainbow_data are updated);
also ensure any corresponding volume names in the main/base compose remain
unchanged (or explicitly set to mainnet names) and/or document/enforce distinct
docker-compose project names (-p) to avoid reuse; pay attention to ENS-related
env vars LABEL_SET_ID and ENSINDEXER_SCHEMA_NAME which assume devnet state so
their DB volume must be isolated.
In `@docker/docker-compose.yml`:
- Around line 34-40: The compose env_file configuration leaves
../apps/ensrainbow/.env.local optional while
apps/ensrainbow/scripts/entrypoint.sh requires DB_SCHEMA_VERSION, LABEL_SET_ID,
and LABEL_SET_VERSION; either make the env file required by changing the
env_file entry for ../apps/ensrainbow/.env.local to required: true, or add the
missing keys (DB_SCHEMA_VERSION, LABEL_SET_ID, LABEL_SET_VERSION) with sensible
defaults into ./.env.docker-compose so a fresh volume has the needed values at
startup; update the env_file block or .env.docker-compose accordingly to ensure
those variables are present for the ENSRainbow entrypoint.
In `@docker/README.md`:
- Line 13: The README contains typos and inaccurate references: change the
command `pnmp devnet` to the correct `pnpm devnet`, fix the sentence describing
`docker/.env.docker-compose` so grammar reads correctly (e.g., "Usually placed
after .env.local so will override it by design"), and update the reference "both
files" (appearing after a list of three env files) to correctly refer to "these
files" or "all three files" so the count matches; apply the same corrections at
the other occurrences noted (lines around the other mentions, e.g., the entries
referenced at 32 and 61).
In `@docker/services/ensadmin.yml`:
- Around line 10-14: Remove the hardcoded environment entries
ENSADMIN_PUBLIC_URL and NEXT_PUBLIC_SERVER_CONNECTION_LIBRARY from the service's
environment block so the values provided by the shared env_file are used
instead; locate the environment section for the ensadmin service and delete the
two lines setting ENSADMIN_PUBLIC_URL and NEXT_PUBLIC_SERVER_CONNECTION_LIBRARY
(following the same pattern used by ensindexer, ensapi, and ensrainbow) so
configuration comes exclusively from docker/.env.docker-compose.
In `@docker/services/postgres.yml`:
- Around line 1-8: Add a Docker healthcheck to the postgres service so Compose
can detect when Postgres is accepting connections: update the postgres service
(container name "postgres") to include a healthcheck using pg_isready (or a
small script that retries) with sensible interval/retries/timeout, and then
update any consumers that currently use depends_on: condition: service_started
to depend on service_healthy instead (or document that they can switch to
service_healthy); reference the postgres service and ensure the healthcheck
command exits non-zero on failure so Compose/Testcontainers
Wait.forHealthCheck() works reliably.
---
Outside diff comments:
In `@packages/integration-test-env/src/orchestrator.ts`:
- Around line 14-17: Update the stale header comment in the file-level doc (the
"Design decisions" block in orchestrator.ts) to reflect that Phase 1 now starts
Postgres and devnet using docker/docker-compose.orchestrator.yml (not the root
docker-compose.yml); edit the sentence that references "root docker-compose.yml"
to mention "docker/docker-compose.orchestrator.yml" and ensure the comment still
conveys that Testcontainers' DockerComposeEnvironment is used so the
orchestrator uses the images/config from that compose file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 25be0611-90c1-4612-a130-b142d1f74705
📒 Files selected for processing (17)
README.mdapps/ensindexer/.env.local.exampleapps/ensrainbow/.env.local.exampledocker-compose.ymldocker/.env.docker-composedocker/README.mddocker/docker-compose.devnet.ymldocker/docker-compose.orchestrator.ymldocker/docker-compose.ymldocker/services/devnet.ymldocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensindexer.ymldocker/services/ensrainbow.ymldocker/services/postgres.ymlpackage.jsonpackages/integration-test-env/src/orchestrator.ts
💤 Files with no reviewable changes (1)
- docker-compose.yml
There was a problem hiding this comment.
Additional Suggestions:
- Import path references incorrect location of docker-compose.yml file after it was moved from root to docker/ directory
- Comment references outdated "root docker-compose.yml" location that no longer exists
- Documentation references docker-compose.yml at monorepo root, but the file was moved to docker/ directory
- README references outdated docker-compose.yml location at monorepo root which has been moved to docker/ directory
- Docker compose commands are missing the
-f docker/docker-compose.ymlflag because the file has been moved from the root to the docker/ directory.
- Docker compose commands missing the
-f docker/docker-compose.ymlflag and documentation references pointing to old file location
- Docker compose commands in README are missing the
-fflag to specify the location of the compose file, causing commands to fail when the files have been moved to the docker/ directory.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docker/README.md (1)
13-32:⚠️ Potential issue | 🟡 MinorFix the remaining README typo and file-count mismatch.
Line 13 has
wil, and Line 32 says “both files” after listing three.env.localfiles.📝 Proposed documentation fix
-| `docker/.env.docker-compose` | Shared env defaults (postgres credentials, internal service URLs). Usually is placed after .env.local so wil override it by design | +| `docker/.env.docker-compose` | Shared env defaults (postgres credentials, internal service URLs). Usually placed after `.env.local`, so it will override it by design. | @@ -Edit both files and set your RPC endpoints (e.g. `RPC_URL_1`, `ALCHEMY_API_KEY`) and any other required values. +Edit these files and set your RPC endpoints (e.g. `RPC_URL_1`, `ALCHEMY_API_KEY`) and any other required values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/README.md` around lines 13 - 32, Fix the README typo and file-count mismatch: correct "wil" to "will" in the sentence about overriding .env.local, and update the phrase "both files" to accurately refer to the three example env files (e.g., "edit the files" or "edit these files") after the three cp lines that list apps/ensindexer/.env.local, apps/ensapi/.env.local, and apps/ensrainbow/.env.local; ensure the instructions consistently reference "these files" or "all three files" so it matches the three cp commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/services/postgres.yml`:
- Around line 3-6: The shared Postgres service currently hardcodes
container_name: postgres and ports: "5432:5432" on the postgres service; remove
those keys from the shared Postgres fragment (delete the container_name and
ports entries for the postgres service) so the fragment no longer forces a fixed
container name or host port binding, and re-add the ports mapping only in the
human-facing compose files (docker-compose.yml and docker-compose.devnet.yml)
where binding 5432:5432 is an intentional local UX choice.
In `@docs/ensnode.io/src/content/docs/docs/contributing/index.mdx`:
- Around line 157-160: Remove the misleading "press `Ctrl + C`" instruction
since the previous step runs Docker Compose with `up -d`; update the paragraph
around the `docker compose -f docker/docker-compose.yml down` command to either
delete the Ctrl+C sentence entirely or explicitly qualify that Ctrl+C only
applies to non-detached runs (i.e., when not using `-d`), so readers are
correctly directed to run `docker compose ... down` for stopping and removing
containers started in detached mode.
---
Duplicate comments:
In `@docker/README.md`:
- Around line 13-32: Fix the README typo and file-count mismatch: correct "wil"
to "will" in the sentence about overriding .env.local, and update the phrase
"both files" to accurately refer to the three example env files (e.g., "edit the
files" or "edit these files") after the three cp lines that list
apps/ensindexer/.env.local, apps/ensapi/.env.local, and
apps/ensrainbow/.env.local; ensure the instructions consistently reference
"these files" or "all three files" so it matches the three cp commands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 61ae11b7-9dbc-4734-9a2e-6e11e20b466e
📒 Files selected for processing (13)
apps/ensindexer/.env.local.exampledocker/README.mddocker/docker-compose.devnet.ymldocker/docker-compose.orchestrator.ymldocker/docker-compose.ymldocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensindexer.ymldocker/services/postgres.ymldocs/ensnode.io/src/content/docs/docs/contributing/index.mdxdocs/ensnode.io/src/content/docs/docs/deploying/docker.mdxpackages/integration-test-env/README.mdpackages/integration-test-env/src/orchestrator.ts
shrugs
left a comment
There was a problem hiding this comment.
i think moving from individual service .env files to a single docker-compose .env helps the mental model a lot. i'd even suggest we avoid sourcing the ./apps/*/.env.local files within the main docker-compose.yml? OR i'd suggest naming it docker-compose.dev.yml so that it is clear that it's for developers to run the local services with their local .env files
regardless, i'm not sure that sourcing both .env.local and .env.docker-compose makes sense — it should be one or the other?
| condition: service_healthy | ||
| postgres: | ||
| condition: service_healthy | ||
| devnet: |
There was a problem hiding this comment.
this depends_on redefines depends_on from the ensindexer service; obviously the devnet dependency must be included, but do we need to re-specify the others?
There was a problem hiding this comment.
well, you are right, we can skip them. but after your comment I decided that it's better to move depends_on from services/ to each docker-compose.*.yml file. It's slightly more verbose (duplicate ensrainbow/postgres in both compose files) but each compose file becomes fully self-describing — no need to chase through extends merge rules to understand what a service waits for.
…irectory, updates docs and cicd-tests
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
docs/ensnode.io/src/content/docs/docs/contributing/index.mdx (1)
169-176:⚠️ Potential issue | 🟡 MinorMake stopping commands match the detached stack that was started.
Both workflows use
up -d, soCtrl + Cis misleading. Also showdown/down -vfor the same compose file the user started, including devnet.📝 Proposed documentation fix
-To stop the running applications, you can press `Ctrl + C` in the terminal where Docker Compose is running. To remove the containers and networks: +To stop a detached stack and remove its containers and networks, run `down` with the same Compose file you used to start it: ```bash +# local devnet +docker compose -f docker/docker-compose.devnet.yml down + +# mainnet/sepolia docker compose -f docker/docker-compose.yml down:::note[Postgres Volume]
-docker compose downwill not delete the Postgres data volume, as it is a named volume. To fully delete it and start from scratch, usedocker compose -f docker/docker-compose.yml down -v.
+docker compose ... downwill not delete the Postgres data volume, as it is a named volume. To fully delete it and start from scratch, use the matching Compose file withdown -v:
+
+bash +docker compose -f docker/docker-compose.devnet.yml down -v +docker compose -f docker/docker-compose.yml down -v +
:::</details> Run this read-only check to verify the stop section mirrors the documented start commands: ```shell #!/bin/bash # Description: Verify Docker Compose start/stop commands in the contributing guide. # Expected: devnet and base start commands both have matching down/down -v guidance, # and the detached workflow does not rely on Ctrl+C. rg -n -C2 'docker compose -f docker/docker-compose(\.devnet)?\.yml (up -d|down|down -v)|Ctrl \+ C' \ docs/ensnode.io/src/content/docs/docs/contributing/index.mdx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ensnode.io/src/content/docs/docs/contributing/index.mdx` around lines 169 - 176, The stop instructions currently mention "Ctrl + C" and only show `docker compose -f docker/docker-compose.yml down`; update the section so it mirrors the detached `up -d` starts by removing the Ctrl + C suggestion and adding explicit `down` and `down -v` commands for both Compose files (`docker/docker-compose.devnet.yml` and `docker/docker-compose.yml`), and update the Postgres Volume note to advise using the matching compose file with `down -v` (show both `docker compose -f docker/docker-compose.devnet.yml down -v` and `docker compose -f docker/docker-compose.yml down -v`) so stop instructions match the documented start commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/envs/.env.docker.example`:
- Around line 4-6: Replace the incorrect word "localization" in the devnet
instruction line "For devnet: no localization needed — defaults are in
.env.docker.devnet." with a clearer phrase such as "local configuration" or
"local setup" so the line reads e.g. "For devnet: no local configuration needed
— defaults are in .env.docker.devnet."; update the same sentence wherever it
appears to ensure consistency.
In `@docker/README.md`:
- Around line 18-21: Add a blank quoted line between the blockquote marker and
the fenced code block that contains the command "docker compose -f
docker/docker-compose.yml config" so the blockquote properly separates from the
triple-backtick fence; update the README block that starts with the quoted line
before the fenced "bash" code block to include an empty line with a leading ">"
to satisfy markdownlint.
In `@docker/services/ensindexer.yml`:
- Around line 10-16: The Docker healthcheck is probing /health which the Ponder
service does not expose; either update the healthcheck in the healthcheck block
to test an existing endpoint (e.g., change the test to curl --fail -s
http://localhost:42069/api/indexing-status) or add a /health route in the Ponder
app that returns a 200 OK (you can delegate it to the same handler as
/api/indexing-status); ensure the chosen endpoint reliably returns HTTP 200 for
healthy state so Docker's health check succeeds.
In `@docker/services/ensrainbow.yml`:
- Around line 13-19: The docker-compose healthcheck for the ensrainbow service
currently sets start_period: 20m which is shorter than the cold-start
expectation in apps/ensindexer/src/lib/ensrainbow/singleton.ts (30+ minutes);
update the ensrainbow healthcheck configuration (healthcheck -> start_period) to
a value >= 30m so Compose does not mark the service unhealthy before it finishes
its cold start and blocks services gated on service_healthy.
In `@docs/ensnode.io/src/content/docs/docs/deploying/docker.mdx`:
- Line 65: Update the note that references the compose file so it points to the
new compose path: replace the mention of `docker-compose.yml` with
`docker/docker-compose.yml` in the sentence about exposed container ports (the
sentence that also references ENSApi's `4334` and other services), leaving the
port numbers and service names unchanged; ensure the wording matches the rest of
the page which already points users to `docker/docker-compose.yml`.
---
Duplicate comments:
In `@docs/ensnode.io/src/content/docs/docs/contributing/index.mdx`:
- Around line 169-176: The stop instructions currently mention "Ctrl + C" and
only show `docker compose -f docker/docker-compose.yml down`; update the section
so it mirrors the detached `up -d` starts by removing the Ctrl + C suggestion
and adding explicit `down` and `down -v` commands for both Compose files
(`docker/docker-compose.devnet.yml` and `docker/docker-compose.yml`), and update
the Postgres Volume note to advise using the matching compose file with `down
-v` (show both `docker compose -f docker/docker-compose.devnet.yml down -v` and
`docker compose -f docker/docker-compose.yml down -v`) so stop instructions
match the documented start commands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7b2c8ab-038d-4547-b37e-b68cd89e8c08
📒 Files selected for processing (21)
.changeset/hip-poems-shave.mdREADME.mdapps/ensindexer/.env.local.exampleapps/ensrainbow/.env.local.exampledocker-compose.ymldocker/README.mddocker/docker-compose.devnet.ymldocker/docker-compose.orchestrator.ymldocker/docker-compose.ymldocker/envs/.env.docker.commondocker/envs/.env.docker.devnetdocker/envs/.env.docker.exampledocker/services/devnet.ymldocker/services/ensadmin.ymldocker/services/ensapi.ymldocker/services/ensindexer.ymldocker/services/ensrainbow.ymldocker/services/postgres.ymldocs/ensnode.io/src/content/docs/docs/contributing/index.mdxdocs/ensnode.io/src/content/docs/docs/deploying/docker.mdxpackage.json
💤 Files with no reviewable changes (1)
- docker-compose.yml
Summary
docker/directory with purpose-specific compose files:docker/services/*.ymlfragments using extends to eliminate duplicationorchestrator.tsto use new docker compose,package.jsonwith build scripts, addeddocker/README.mdwith usage docs for developersWhy
Resolves #1923
Testing
docker compose configused to verify resolved output after restructureNotes for Reviewer (Optional)
Maybe rename
postgresservice toensdb? not sure, probably ensdb is word for database schema, not for postgres serviceenv_file is not defined in services/*.yml — each compose file declares its own env_file list explicitly to control order and
requiredparamvolumes: must be re-declared in every compose file
Initially wanted to add docker-compose.yml to root using symlink, but dropped it. it broke
extendspath resolution when Docker Compose resolved paths relative to the symlink location rather than the real file :(Pre-Review Checklist (Blocking)
[x] This PR does not introduce significant changes and is low-risk to review quickly.
[x] Relevant changesets are included (or are not required)