Skip to content

Comments

Migrations from entrypoints#190

Closed
endersonmaia wants to merge 3 commits intoprerelease/v2-alphafrom
refactor/migrations-from-entrypoints
Closed

Migrations from entrypoints#190
endersonmaia wants to merge 3 commits intoprerelease/v2-alphafrom
refactor/migrations-from-entrypoints

Conversation

@endersonmaia
Copy link
Contributor

@endersonmaia endersonmaia commented Mar 18, 2025

This pull request includes several updates to the Docker Compose configurations for various services. The main changes involve updating the PostgreSQL image and simplifying the service definitions by merging migration steps into the main service entrypoints.

Updates to PostgreSQL image:

Simplification of service definitions:

@endersonmaia endersonmaia requested a review from tuler March 18, 2025 13:34
@endersonmaia endersonmaia self-assigned this Mar 18, 2025
@changeset-bot
Copy link

changeset-bot bot commented Mar 18, 2025

⚠️ No Changeset found

Latest commit: 8967565

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2025

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 27.43% 293 / 1068
🔵 Statements 27.41% 298 / 1087
🔵 Functions 27.27% 51 / 187
🔵 Branches 25.74% 121 / 470
File CoverageNo changed files found.
Generated in workflow #382 for commit 8967565 by the Vitest Coverage Report Action

@tuler
Copy link
Member

tuler commented Mar 18, 2025

For the reasons explained at https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/ I highly prefer to use exec form only, and not use ENTRYPOINT to run database migrations through shell.
I don't think it's a correct usage of ENTRYPOINT, and I think it's fine to use a container to do database initialization.

Another strategy I see around is the software itself takes care of database initialization, which has the advantage of guaranteeing the correct migration is executed. But rollups-node and rollups-graphql are not doing that.

@endersonmaia
Copy link
Contributor Author

For the reasons explained at https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/ I highly prefer to use exec form only, and not use ENTRYPOINT to run database migrations through shell. I don't think it's a correct usage of ENTRYPOINT, and I think it's fine to use a container to do database initialization.

I agree with the reasons mentioned in the post, but mostly it's ok for single purpose containers, that's not our case. We have an SDK container that has a lot of auxiliary tools and services inside, the container itself is used to bundle everything and ensure compatibility with every tool for the cartesi environment.

What I'm trying to achieve with removing those one-shot containers that are used for migrations (and even createdb) is reduce the noise exposed to the cartesi/cli user when running cartesi start and also the number of containers to debug when necessary via docker compose directly.

If you agree, I think we could enhance the cartesi/sdk container image with en entrypoint that could be used from cartesi/cli and even calling docker run cartesi/sdk directly if necessary.

Another strategy I see around is the software itself takes care of database initialization, which has the advantage of guaranteeing the correct migration is executed. But rollups-node and rollups-graphql are not doing that.

For that, I suggest talking to those project's teams and issue a feature request if we think it's


I'm closing this and will get back to it if we decide to have an entrypoint defined at cartesi/sdk and reuse if from the cli.

@tuler
Copy link
Member

tuler commented Mar 18, 2025

reduce the noise exposed to the cartesi/cli user when running cartesi start

Noise of the pull or the start itself?
Start only monitors services, not the one-shot.
And pull has to pull anyway once, and I believe layers are only fetched once.

@endersonmaia
Copy link
Contributor Author

Noise of the pull or the start itself?

Yes.

Start only monitors services, not the one-shot.

That's fine.

And pull has to pull anyway once, and I believe layers are only fetched once.

For example:

✗ cartesi-dev rollups start --services graphql,espresso
WARNING: default block is set to 'latest', production configuration will likely use 'finalized'
[+] Pulling 12/12
 ✔ espresso_reader_migration Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                    0.0s
 ✔ graphql_database_migration Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                   0.0s
 ✔ espresso Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                                     0.0s
 ✔ rollups-node Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                                 0.0s
 ✔ espresso_database_creator Skipped - Image is already being pulled by graphql_database_creator                                                                                                                                                                                                                 0.0s
 ✔ graphql Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                                      0.0s
 ✔ database Skipped - Image is already being pulled by graphql_database_creator                                                                                                                                                                                                                                  0.0s
 ✔ rollups-node-migration Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                       0.0s
 ✔ espresso_reader Skipped - Image is already being pulled by anvil                                                                                                                                                                                                                                              0.0s
 ✔ proxy Pulled
....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants