[Fixes #242] Allow starting one service at a time#249
Conversation
📝 WalkthroughWalkthroughThis pull request adds startup automation and documentation improvements. A new Bash script at 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ddev/commands/host/up:
- Around line 6-8: The script currently invokes START_UP_SERVICE=${SERVICE} ddev
start before validating SERVICE, so passing an invalid argument (e.g., `ddev up
typo`) will boot the stack then fail; move or add validation of SERVICE (ensure
SERVICE is one of the allowed values or matches existing service names, and
treat empty/malformed values) before the START_UP_SERVICE=${SERVICE} ddev start
call, and if invalid print the usage/error and exit non-zero; update both the
main startup path and the alternate branch that covers lines 12-25 so validation
happens before any ddev start invocation (refer to the SERVICE variable,
START_UP_SERVICE assignment, and the invalid-argument branch to locate places to
change).
In @.ddev/config.hooks.yaml:
- Line 4: The post-start hook currently masks installation failures because the
exec entry with "[ ! -d node_modules ] && npm ci || true" always returns
success; remove the trailing "|| true" and change the hook so it runs npm ci
only when node_modules is missing but does not swallow errors (e.g. use a shell
conditional that runs npm ci when node_modules is absent and lets npm's non-zero
exit code propagate); update the exec entry (the post-start exec command string)
accordingly so failed installs surface instead of being ignored.
🪄 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: CHILL
Plan: Pro
Run ID: 57d36cd4-67fc-434c-8fa7-4792c5a666d0
📒 Files selected for processing (3)
.ddev/commands/host/up.ddev/config.hooks.yamlREADME.project.md
| SERVICE=${1:-all} | ||
|
|
||
| START_UP_SERVICE=${SERVICE} ddev start |
There was a problem hiding this comment.
Validate SERVICE before starting DDEV
Line 8 triggers ddev start before the invalid-argument branch at Lines 23-25. ddev up typo still boots the stack, then exits with usage. Fail fast before startup.
Suggested patch
SERVICE=${1:-all}
-START_UP_SERVICE=${SERVICE} ddev start
+case "$SERVICE" in
+ nextjs|storybook|all) ;;
+ *)
+ echo "Usage: ddev up [nextjs|storybook|all]"
+ exit 1
+ ;;
+esac
+
+START_UP_SERVICE="$SERVICE" ddev start
echo "--- Selective PM2 Startup: $SERVICE ---"
-case $SERVICE in
+case "$SERVICE" in
nextjs)
ddev exec pm2 start --name next node_modules/.bin/next -- dev --webpack
;;
@@
- *)
- echo "Usage: ddev up [nextjs|storybook|all]"
- exit 1
- ;;
esacAlso applies to: 12-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/commands/host/up around lines 6 - 8, The script currently invokes
START_UP_SERVICE=${SERVICE} ddev start before validating SERVICE, so passing an
invalid argument (e.g., `ddev up typo`) will boot the stack then fail; move or
add validation of SERVICE (ensure SERVICE is one of the allowed values or
matches existing service names, and treat empty/malformed values) before the
START_UP_SERVICE=${SERVICE} ddev start call, and if invalid print the
usage/error and exit non-zero; update both the main startup path and the
alternate branch that covers lines 12-25 so validation happens before any ddev
start invocation (refer to the SERVICE variable, START_UP_SERVICE assignment,
and the invalid-argument branch to locate places to change).
| service: web | ||
| # pm2 does not live in the node path, so have to call binary directly | ||
| - exec: pm2 start --name storybook node_modules/.bin/storybook -- dev --no-open -p 6006 | ||
| - exec: 'sh -c "[ ! -d node_modules ] && npm ci || true"' |
There was a problem hiding this comment.
Don’t mask dependency install failures in post-start
Line 4 always succeeds due to || true, even when npm ci fails with missing node_modules. That can cascade into opaque startup failures later in the same post-start flow.
Suggested patch
- - exec: 'sh -c "[ ! -d node_modules ] && npm ci || true"'
+ - exec: |
+ sh -c '
+ if [ ! -d node_modules ]; then
+ npm ci || {
+ echo "npm ci failed in web service. Run `ddev frontend install` and retry."
+ exit 1
+ }
+ fi
+ '🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.ddev/config.hooks.yaml at line 4, The post-start hook currently masks
installation failures because the exec entry with "[ ! -d node_modules ] && npm
ci || true" always returns success; remove the trailing "|| true" and change the
hook so it runs npm ci only when node_modules is missing but does not swallow
errors (e.g. use a shell conditional that runs npm ci when node_modules is
absent and lets npm's non-zero exit code propagate); update the exec entry (the
post-start exec command string) accordingly so failed installs surface instead
of being ignored.
|
@mdrayer I know you were encountering slowness with ddev for MySI. Can you test locally whether this approach addresses any of the issues you were seeing? |
2 participants
ddev startstill works, but you can alternately useddev up nextjsorddev up storybookto start with a single service.