cal/major-domo-database
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: drop :latest tag from CI, make worker count configurable #116

Merged
cal merged 2 commits from fix/ci-tags-and-workers into main 2026-04-09 16:32:56 +00:00
Conversation 1 Commits 2 Files Changed 2 +2 -3
cal commented 2026-04-09 13:42:10 +00:00
Owner
Copy Link

Summary

  • Remove :latest Docker tag from CI builds to match Paper Dynasty convention
  • CI now pushes :version + :environment only (:dev or :production)
  • Add WEB_WORKERS env var to Dockerfile (default 2), overridable via docker-compose

Note

Akamai docker-compose currently pulls :latest — must be updated to :production before the next CalVer release. Tracked separately.

Test plan

  • Merge, force-update dev tag, verify CI build succeeds
  • Verify Docker Hub :dev image is updated, no new :latest pushed
  • Deploy to dev, verify container starts with 2 workers

🤖 Generated with Claude Code

## Summary - Remove `:latest` Docker tag from CI builds to match Paper Dynasty convention - CI now pushes `:version` + `:environment` only (`:dev` or `:production`) - Add `WEB_WORKERS` env var to Dockerfile (default 2), overridable via docker-compose ## Note Akamai docker-compose currently pulls `:latest` — must be updated to `:production` before the next CalVer release. Tracked separately. ## Test plan - [ ] Merge, force-update `dev` tag, verify CI build succeeds - [ ] Verify Docker Hub `:dev` image is updated, no new `:latest` pushed - [ ] Deploy to dev, verify container starts with 2 workers 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 1 commit 2026-04-09 13:42:11 +00:00 
Remove :latest Docker tag to match Paper Dynasty convention — only
:version and :environment tags are pushed. Add WEB_WORKERS env var
to Dockerfile (default 2) so prod can override via docker-compose.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-04-09 13:45:50 +00:00
Claude requested changes 2026-04-09 13:46:54 +00:00
Dismissed
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • .gitea/workflows/docker-build.yml (modified)
  • Dockerfile (modified)

Findings

Correctness

Dockerfile: shell form CMD loses PID 1 signal handling

The change from exec form to shell form introduces a signal delivery regression:

# Old (exec form) — uvicorn is PID 1, receives SIGTERM directly
CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "80", "--workers", "2"]

# New (shell form) — /bin/sh is PID 1, SIGTERM goes to shell, not uvicorn
ENV WEB_WORKERS=2
CMD uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS

When docker stop is issued, SIGTERM goes to PID 1. With shell form, PID 1 is /bin/sh, which does not forward signals to child processes. Uvicorn never receives SIGTERM and gets SIGKILL after Docker's stop timeout (10s). This bypasses uvicorn's graceful shutdown — in-flight requests are cut off and worker processes are killed hard.

Fix: Use exec form with an explicit shell invocation and exec to replace the shell with uvicorn:

ENV WEB_WORKERS=2
CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS"]

The exec prefix replaces the shell process with uvicorn, making uvicorn PID 1 and restoring correct signal delivery.

Security

  • No issues found.

Style & Conventions

  • CI change (removing :latest from push tags and summary) is clean and correct.
  • The PR note about updating akamai docker-compose to pull :production before next release is appropriate — important operational prerequisite.

Suggestions

  • None beyond the required fix above.

Verdict: REQUEST_CHANGES

One correctness issue: the shell form CMD causes SIGTERM to be delivered to the intermediate shell rather than uvicorn, preventing graceful shutdown. Use CMD ["sh", "-c", "exec uvicorn ..."] to preserve PID 1 behavior while still expanding $WEB_WORKERS. The CI :latest removal is clean and correct.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `.gitea/workflows/docker-build.yml` (modified) - `Dockerfile` (modified) ### Findings #### Correctness **Dockerfile: shell form CMD loses PID 1 signal handling** The change from exec form to shell form introduces a signal delivery regression: ```dockerfile # Old (exec form) — uvicorn is PID 1, receives SIGTERM directly CMD ["uvicorn", "app.main:app", "--host", "0.0.0.0", "--port", "80", "--workers", "2"] # New (shell form) — /bin/sh is PID 1, SIGTERM goes to shell, not uvicorn ENV WEB_WORKERS=2 CMD uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS ``` When `docker stop` is issued, SIGTERM goes to PID 1. With shell form, PID 1 is `/bin/sh`, which does not forward signals to child processes. Uvicorn never receives SIGTERM and gets SIGKILL after Docker's stop timeout (10s). This bypasses uvicorn's graceful shutdown — in-flight requests are cut off and worker processes are killed hard. **Fix:** Use exec form with an explicit shell invocation and `exec` to replace the shell with uvicorn: ```dockerfile ENV WEB_WORKERS=2 CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS"] ``` The `exec` prefix replaces the shell process with uvicorn, making uvicorn PID 1 and restoring correct signal delivery. #### Security - No issues found. #### Style & Conventions - CI change (removing `:latest` from push tags and summary) is clean and correct. - The PR note about updating akamai docker-compose to pull `:production` before next release is appropriate — important operational prerequisite. #### Suggestions - None beyond the required fix above. ### Verdict: REQUEST_CHANGES One correctness issue: the shell form `CMD` causes SIGTERM to be delivered to the intermediate shell rather than uvicorn, preventing graceful shutdown. Use `CMD ["sh", "-c", "exec uvicorn ..."]` to preserve PID 1 behavior while still expanding `$WEB_WORKERS`. The CI `:latest` removal is clean and correct. --- *Automated review by Claude PR Reviewer*
Claude added
ai-changes-requested
 and removed
ai-reviewing
 labels 2026-04-09 13:47:11 +00:00
Claude added the
ai-working
 label 2026-04-09 14:00:50 +00:00
cal added 1 commit 2026-04-09 14:01:26 +00:00 
Shell form CMD makes /bin/sh PID 1 — SIGTERM from docker stop goes to
the shell, not uvicorn, causing SIGKILL after the stop timeout instead
of graceful shutdown. Using CMD ["sh", "-c", "exec uvicorn ..."] lets
the shell expand $WEB_WORKERS then exec-replaces itself with uvicorn,
restoring correct signal delivery.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude commented 2026-04-09 14:01:34 +00:00
Collaborator
Copy Link

Addressed review feedback in 6efba47:

Fixed: shell form CMD → exec form CMD

Changed Dockerfile line 34 from:

CMD uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS

to:

CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS"]

The exec prefix replaces the shell with uvicorn, making uvicorn PID 1 so it receives SIGTERM directly from docker stop and can shut down gracefully rather than being SIGKILLed after the stop timeout.

Addressed review feedback in 6efba47: **Fixed: shell form CMD → exec form CMD** Changed `Dockerfile` line 34 from: ```dockerfile CMD uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS ``` to: ```dockerfile CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port 80 --workers $WEB_WORKERS"] ``` The `exec` prefix replaces the shell with uvicorn, making uvicorn PID 1 so it receives SIGTERM directly from `docker stop` and can shut down gracefully rather than being SIGKILLed after the stop timeout.
Claude removed the
ai-working
ai-changes-requested
 labels 2026-04-09 14:01:41 +00:00
Claude added the
ai-reviewing
 label 2026-04-09 14:15:50 +00:00
Claude reviewed 2026-04-09 14:17:14 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • .gitea/workflows/docker-build.yml (modified)
  • Dockerfile (modified)

Findings

Correctness

  • PID 1 handling (fixed): Previous review flagged that CMD ["sh", "-c", "uvicorn ..."] without exec would lose PID 1 — the shell would sit at PID 1 and SIGTERM would never reach uvicorn. The updated CMD correctly uses exec uvicorn, which replaces the shell process so uvicorn becomes PID 1 and receives SIGTERM directly. Graceful shutdown now works correctly.
  • Variable expansion: $WEB_WORKERS is set via ENV WEB_WORKERS=2 before the CMD — sh expands it correctly at container start. Overridable via docker-compose environment section as intended.
  • CI tag consistency: :latest removed from both the tags: block (line 61) and the Build Summary step (line 74). Both are consistent — no stale :latest reference remains.

Security

No issues found. WEB_WORKERS exposes no sensitive data. No hardcoded credentials introduced.

Style & Conventions

No issues found. Dockerfile follows existing patterns. YAML changes are minimal.

Suggestions

  • Minor pre-existing: Dockerfile still lacks a trailing newline (\ No newline at end of file). Not introduced by this PR — out of scope.

Verdict: COMMENT

Approved. The PID 1 fix from the previous review has been applied correctly (exec uvicorn). CI :latest removal is clean and consistent. The WEB_WORKERS env var addition is correct and the default of 2 is reasonable.

Reminder per PR note: Akamai docker-compose.prod.yml must be updated to pull :production instead of :latest before the next CalVer release.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `.gitea/workflows/docker-build.yml` (modified) - `Dockerfile` (modified) ### Findings #### Correctness - **PID 1 handling (fixed)**: Previous review flagged that `CMD ["sh", "-c", "uvicorn ..."]` without `exec` would lose PID 1 — the shell would sit at PID 1 and SIGTERM would never reach uvicorn. The updated CMD correctly uses `exec uvicorn`, which replaces the shell process so uvicorn becomes PID 1 and receives SIGTERM directly. Graceful shutdown now works correctly. - **Variable expansion**: `$WEB_WORKERS` is set via `ENV WEB_WORKERS=2` before the CMD — sh expands it correctly at container start. Overridable via `docker-compose` environment section as intended. - **CI tag consistency**: `:latest` removed from both the `tags:` block (line 61) and the Build Summary step (line 74). Both are consistent — no stale `:latest` reference remains. #### Security No issues found. `WEB_WORKERS` exposes no sensitive data. No hardcoded credentials introduced. #### Style & Conventions No issues found. Dockerfile follows existing patterns. YAML changes are minimal. #### Suggestions - Minor pre-existing: Dockerfile still lacks a trailing newline (`\ No newline at end of file`). Not introduced by this PR — out of scope. ### Verdict: COMMENT Approved. The PID 1 fix from the previous review has been applied correctly (`exec uvicorn`). CI `:latest` removal is clean and consistent. The `WEB_WORKERS` env var addition is correct and the default of 2 is reasonable. **Reminder per PR note**: Akamai `docker-compose.prod.yml` must be updated to pull `:production` instead of `:latest` before the next CalVer release. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewed
 label 2026-04-09 14:17:48 +00:00
Claude approved these changes 2026-04-09 16:32:34 +00:00
Claude left a comment
Collaborator
Copy Link

Reviewed and approved. Merging now.

Reviewed and approved. Merging now.
cal merged commit 29f9875718 into main 2026-04-09 16:32:56 +00:00
cal deleted branch fix/ci-tags-and-workers 2026-04-09 16:32:56 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Claude
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-failed
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/major-domo-database#116
No description provided.
Delete Branch "fix/ci-tags-and-workers"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?