-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support passing in replica URLs in JSON format #5199
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
for more information, see https://pre-commit.ci
Docker builds report
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5199 +/- ##
==========================================
+ Coverage 97.50% 97.51% +0.01%
==========================================
Files 1224 1227 +3
Lines 42623 42621 -2
==========================================
+ Hits 41558 41561 +3
+ Misses 1065 1060 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
api/app/settings/common.py
Outdated
) | ||
|
||
if REPLICA_DATABASE_URLS_JSON: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: REPLICA_DATABASE_URLS_JSON
is not JSON but python here.. Maybe we can change it something like:
REPLICA_DATABASE_URLS_JSON = env.str("REPLICA_DATABASE_URLS_JSON")
if REPLICA_DATABASE_URLS_JSON:
REPLICA_DATABASE_URLS= json.loads(REPLICA_DATABASE_URLS)
for more information, see https://pre-commit.ci
Thanks for the review! I'll merge this once I can generate these variables directly from Helm. |
I spent way too much time trying to use this from a Helm chart and decided it's not a feasible approach. I'll open a separate PR with a different method. |
When setting
REPLICA_DATABASE_URLS
orCROSS_REGION_REPLICA_DATABASE_URLS
, users need to know if their passwords/URLs contain a comma, and setREPLICA_DATABASE_URLS_DELIMITER
accordingly. This is annoying and not always possible, especially in environments where users might not ever have access to the raw database password such as Kubernetes secrets.Instead of having users that don't know their passwords go through trial and error, we now accept the URLs as a JSON-formatted array and avoid needing to specify a delimiter. The plan is to use this new variable to make it easier to pass arbitrary replica URLs from Helm values.