speech_to_phrase: Shut down cleanly on stop - #4800
Conversation
speech-to-phrase has no SIGTERM handler, so s6's default SIGTERM kills the process and the container exits with code 143, which newer Supervisor versions flag as an unclean shutdown. It does catch KeyboardInterrupt and exit 0 on SIGINT, so tell s6 to stop the service with SIGINT via the down-signal file. Verified with the full s6 init: docker stop completes in ~3s, the service log shows a clean stop, and the container exits with code 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe add-on version changes to 1.4.5. Startup health checks run every 5 seconds before switching to 30-second checks. The s6 service sends ChangesSpeech-to-phrase runtime updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The service now stops with SIGINT for a clean exit, with the app version updated accordingly; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
With the default 30s healthcheck interval the app can sit in the "Starting" state for half a minute after it is already serving. Use --start-interval=5s so readiness is detected within seconds once training completes and the Wyoming port opens. Verified: container reports healthy 8s after start (including forced training), down from up to ~40s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds clean shutdown behavior and faster startup readiness detection for Speech-to-Phrase.
Changes:
- Sends SIGINT when stopping the s6 service.
- Probes readiness every five seconds during startup.
- Bumps the app version to 1.4.5 and updates the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
speech_to_phrase/rootfs/etc/s6-overlay/s6-rc.d/speech-to-phrase/down-signal |
Configures clean SIGINT shutdown. |
speech_to_phrase/Dockerfile |
Adds the startup healthcheck interval. |
speech_to_phrase/config.yaml |
Bumps the app version. |
speech_to_phrase/CHANGELOG.md |
Documents the release changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
What this PR changes
down-signalfile so the speech-to-phrase service is stopped with SIGINT instead of SIGTERM.--start-interval=5s, normal interval stays 30 s).Background
Clean shutdown: speech-to-phrase installs no SIGTERM handler, so on stop the process dies from the default signal action and the container exits with code 143, which newer Supervisor versions flag:
It does, however, catch
KeyboardInterruptaround its asyncio entrypoint — so SIGINT produces a clean exit 0 today, no upstream change required. s6-rc'sdown-signalfile makes s6 deliver SIGINT on service stop. Verified against the real s6 init in the built image:docker stopcompletes in ~3 s, s6 logs "service speech-to-phrase successfully stopped", and the container exit code is 0 (previously 143).Faster startup detection: the app only opens its Wyoming port after the initial training pass, and with the default 30 s healthcheck interval it could sit in "Starting" for half a minute after it was already serving. With
--start-interval=5sthe built image reports healthy 8 s after container start (including forced training), down from up to ~40 s. Requires Docker Engine ≥ 25 (HAOS ships well past that) and hadolint ≥ v2.14 in CI — see #4802, which should merge first.Longer term, a SIGTERM handler in python-wyoming's
AsyncServerfixes all Wyoming apps at once — submitted as OHF-Voice/wyoming#55; thedown-signalmakes this app well-behaved now and becomes redundant-but-harmless once that ships. Companion PR for Whisper: #4801.🤖 Generated with Claude Code