Skip to content

Conversation

@endersonmaia
Copy link
Contributor

@endersonmaia endersonmaia commented Mar 13, 2025

TL;DR

With first 2 items, already reduced from ~30s to ~9.5s on my environment.


This pull request includes significant changes to the apps/cli directory, focusing on removing the prompt service from various Docker Compose files and adjusting health check configurations.

Removal of prompt service:

Health check configuration adjustments:

@changeset-bot
Copy link

changeset-bot bot commented Mar 13, 2025

⚠️ No Changeset found

Latest commit: be6865d

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 13, 2025

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 28.14% 293 / 1041
🔵 Statements 28.24% 298 / 1055
🔵 Functions 30.17% 51 / 169
🔵 Branches 26.19% 121 / 462
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/cli/src/commands/rollups/start.ts 0% 0% 0% 0% 7-187
Generated in workflow #338 for commit be6865d by the Vitest Coverage Report Action

@tuler
Copy link
Member

tuler commented Mar 13, 2025

My idea for the replacement of the prompt was to write a typescript library that monitors the up of a docker compose by listening to events of docker compose events json output. The library user would register containers of "interest" and be notified when they get healthy. Then a client of this library could use some presentation framework like https://listr2.kilic.dev to present prompt messages of each container of interest.
The original output of docker compose up --detached could be suppressed from stdout, as the new output would be more user friendly.

@endersonmaia
Copy link
Contributor Author

My idea for the replacement of the prompt was to write a typescript library that monitors the up of a docker compose by listening to events of docker compose events json output. The library user would register containers of "interest" and be notified when they get healthy. Then a client of this library could use some presentation framework like https://listr2.kilic.dev to present prompt messages of each container of interest. The original output of docker compose up --detached could be suppressed from stdout, as the new output would be more user friendly.

Please, open an issue to track this.

@endersonmaia endersonmaia requested a review from tuler March 13, 2025 18:16
@endersonmaia endersonmaia force-pushed the feature/improve-time-to-start branch from 86691a1 to 1c1db1d Compare March 13, 2025 18:17
image: debian:bookworm-slim
environment:
PROMPT_TXT_01_ANVIL: "Anvil running at http://localhost:8545"
volumes:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why db volume in anvil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it's the container we have always running that uses the cartesi/sdk image which has the initialized database

ugly, I know

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having another container with a sleep infinity looked worse for me

do you have any other idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extend postgres image itself with the db?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean, make the cartesi/sdk based on postgres?

or maintain another container image cartesi/postgres ?

@endersonmaia endersonmaia force-pushed the feature/improve-time-to-start branch from b6ab38c to 1d1dd72 Compare March 13, 2025 18:55
@endersonmaia endersonmaia force-pushed the feature/improve-time-to-start branch from 1d1dd72 to be6865d Compare March 13, 2025 18:58
@tuler tuler force-pushed the feature/cli-run-v2 branch 4 times, most recently from caf7a2f to c2a395d Compare March 15, 2025 16:05
@endersonmaia
Copy link
Contributor Author

Replaced by #179

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants