fix(docker): stop injecting secure=true into CLICKHOUSE_URL#3190
fix(docker): stop injecting secure=true into CLICKHOUSE_URL#3190mangit955 wants to merge 1 commit intotriggerdotdev:mainfrom
Conversation
|
|
Hi @mangit955, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe change removes conditional logic from a shell script that previously ensured the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
| # Use the provided ClickHouse URL directly | ||
| export GOOSE_DBSTRING="$CLICKHOUSE_URL" |
There was a problem hiding this comment.
🚩 Breaking change for self-hosting users who relied on implicit secure=true
The old code automatically appended secure=true to CLICKHOUSE_URL if no secure= parameter was present. Self-hosting users who previously omitted the secure parameter from their CLICKHOUSE_URL (relying on the entrypoint to inject it) will now have their goose migrations connect without TLS. This could cause migration failures if their ClickHouse instance requires secure connections. The hosting/docker/.env.example:65 already includes ?secure=false explicitly, and hosting/docker/webapp/docker-compose.yml:71 defaults to ?secure=false, suggesting the self-hosting defaults are fine. However, users with production ClickHouse behind TLS who relied on the auto-injection will need to update their CLICKHOUSE_URL to include secure=true.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Hi! Would it be possible for a maintainer to vouch for me so I can reopen the PR? I verified the issue locally with the Docker setup and confirmed that removing the automatic |
|
From a community member, thank you for this PR! I was able to replicate and confirm it fixes the issue. Definitely need to get this merged. |
Thanks for confirming the fix! Since the PR was auto-closed due to the vouch requirement, would a maintainer be able to vouch for me so I can reopen the PR? I'm happy to make any changes needed or update the implementation if required. |
Closes #3184
✅ Checklist
Testing
Steps taken to test the change:
Error: Unknown URL parameters: securesecure=trueinto the ClickHouse connection string.Unknown URL parameters: secureerror occurs.Changelog
Fix Docker entrypoint script to stop automatically injecting
secure=trueintoCLICKHOUSE_URL.Some ClickHouse clients (including the Node client used by Trigger.dev) do not support this parameter, which causes startup failures.
The script now uses the provided
CLICKHOUSE_URLdirectly.Screenshots
N/A (backend / Docker configuration change)