fix(docker): clickhouse healthcheck#2462
Conversation
|
WalkthroughUpdated the ClickHouse healthcheck in hosting/docker/webapp/docker-compose.yml to reference environment variables for credentials. The --user flag now uses ${CLICKHOUSE_USER:-default} and --password uses ${CLICKHOUSE_PASSWORD:-password}, replacing previously hard-coded values. No other healthcheck parameters or file sections were modified. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
hosting/docker/webapp/docker-compose.yml (2)
157-157: Optional: avoid exposing password in argv or use HTTP /ping if allowed.If your ClickHouse HTTP interface doesn’t require auth, consider this simpler check to avoid placing the password in the process list:
- test: ["CMD", "clickhouse-client", "--host", "localhost", "--port", "9000", "--user", "${CLICKHOUSE_USER:-default}", "--password", "${CLICKHOUSE_PASSWORD:-password}", "--query", "SELECT 1"] + test: ["CMD", "curl", "-fsS", "http://localhost:8123/ping"]If auth is required on HTTP, keeping the current native protocol check is fine.
Please confirm whether your ClickHouse config permits unauthenticated /ping; if not, keep the native check.
149-151: Naming consistency note (no action required).You’re mapping CLICKHOUSE_ADMIN_USER/PASSWORD from CLICKHOUSE_USER/PASSWORD and using CLICKHOUSE_USER/PASSWORD in the healthcheck. That’s fine, but document this in your env setup so operators know to set CLICKHOUSE_USER/PASSWORD as the single source of truth.
Also applies to: 157-157
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
hosting/docker/webapp/docker-compose.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (22)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (1)
hosting/docker/webapp/docker-compose.yml (1)
157-157: LGTM — healthcheck now respects overridden ClickHouse credentials.Switching from hard-coded defaults to compose-time interpolation fixes the original issue.
|
Thanks for this 🙏 |
The healthcheck for clickhouse used the default username/password and would cause an error if you changed it
✅ Checklist