-
Notifications
You must be signed in to change notification settings - Fork 134
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
Fixed continuous config appending #83
Conversation
On Heroku the filesystem is ephemeral, so previous run's configs won't exist any more. |
Ah there's a dupe of this, #70 which gives an example scenario. |
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.
The change described in the title is fine, but I am suspicious of the change in connection string parsing. As we don't have tests for this, I don't know how this was validated or how it would work with odd input.
I suggest breaking that change into another pr so we can test and qualify it without blocking the config appending.
c9033f6
to
098da53
Compare
@gregburek I split this into two separate pull requests. |
The users.txt file must be appended to as multiple postgres dbs in POSTGRES_URLS, we need to add all the creds to the users.txt file, not reset it for every entry in POSTGRES_URLS.
@@ -86,7 +86,7 @@ connect = $DB_HOST:$DB_PORT | |||
retry = ${PGBOUNCER_CONNECTION_RETRY:-"no"} | |||
EOFEOF | |||
|
|||
cat >> /app/vendor/pgbouncer/users.txt << EOFEOF | |||
cat > /app/vendor/pgbouncer/users.txt << EOFEOF |
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.
yeah this is inside the for loop, so it must be append only. Fixed in #89
Otherwise running `start-stunnel <COMMAND>` multiple times (such as in a one-off dyno when running a command and then killing it) will result in an stunnel.conf that has invalid syntax due to the global options being appended after the per-server header. Based on: heroku/heroku-buildpack-pgbouncer#83
Fixed the issue where the stunnel-pgbouncer.conf, user.txt and pgbouncer.ini files are being repeatedly appended on server restarts, instead of erasing and restarting.
Also fixed the DATABASE_URL parse to not fail on missing passwords.