Skip to content

Add HEALTHCHECK to token-spy Dockerfile#372

Open
reo0603 wants to merge 3 commits intoLight-Heart-Labs:mainfrom
reo0603:feat/token-spy-healthcheck
Open

Add HEALTHCHECK to token-spy Dockerfile#372
reo0603 wants to merge 3 commits intoLight-Heart-Labs:mainfrom
reo0603:feat/token-spy-healthcheck

Conversation

@reo0603
Copy link
Contributor

@reo0603 reo0603 commented Mar 18, 2026

Summary

  • Add Docker HEALTHCHECK directive to token-spy service for consistency with other Python services

Motivation

Token-spy was the only remaining Python service without a HEALTHCHECK directive. All other services (dashboard-api, APE, privacy-shield, comfyui) have health monitoring configured.

Changes

  • Added HEALTHCHECK directive between EXPOSE and CMD
  • Uses Python's urllib to check the /health endpoint (port 8080)
  • Matches the pattern used in APE service (Python-based healthcheck)

Testing

  • HEALTHCHECK will run automatically when container starts
  • Checks every 30s with 5s timeout
  • 5s start period allows service initialization
  • 3 retries before marking unhealthy

Copy link
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

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

Conflicts with PR #304

This PR overlaps directly with #304, which also adds a HEALTHCHECK to the token-spy Dockerfile (and additionally covers APE). Merging both will produce a conflict on the same lines. Coordinate — either close this in favor of #304, or merge this first and rebase #304.

Hardcoded port ignores UVICORN_PORT

The healthcheck hardcodes port 8080, but the CMD at the bottom of the same Dockerfile uses ${UVICORN_PORT:-8080}. If UVICORN_PORT is set to a non-8080 value, uvicorn listens on that port while the healthcheck still probes 8080 — container gets marked unhealthy. PR #304 handles this correctly with shell variable expansion.

Minor: timeout inconsistency

This PR uses --timeout=5s while other Python-based healthchecks (privacy-shield, #304) use --timeout=10s. Spawning a Python interpreter is slower than curl, so 10s is safer.

@Lightheartdevs
Copy link
Collaborator

What's needed to get this merged:

  1. Fix the hardcoded port — use shell variable expansion so healthcheck respects UVICORN_PORT:
    HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
        CMD python3 -c "import urllib.request; urllib.request.urlopen('http://localhost:${UVICORN_PORT:-8080}/health')" || exit 1
  2. Change --timeout=5s to --timeout=10s for consistency with other Python-based healthchecks
  3. Coordinate with feat: add health checks to APE and token-spy Dockerfiles #304 which also touches this Dockerfile — now that Add HEALTHCHECK to APE Dockerfile #368 (APE healthcheck) is merged, feat: add health checks to APE and token-spy Dockerfiles #304 only needs the token-spy side. Consider closing this in favor of feat: add health checks to APE and token-spy Dockerfiles #304 if they fix the port issue there.

@reo0603
Copy link
Contributor Author

reo0603 commented Mar 18, 2026

Updated to address the review feedback:

  1. Fixed hardcoded port — now uses ${UVICORN_PORT:-8080} so the healthcheck respects the environment variable
  2. Increased timeout to 10s — matches other Python-based healthchecks (spawning Python interpreter is slower than curl)

The healthcheck now works correctly regardless of which port uvicorn is configured to use.

@reo0603 reo0603 requested a review from Lightheartdevs March 18, 2026 14:03
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.

2 participants