Alternatively accept PG* env vars for database configuration #702
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e473535 to
4c7a74b
Compare
afce487 to
64837de
Compare
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Dec 24, 2024
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. [1] riverqueue/river#702
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Dec 24, 2024
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. Fixes #249. [1] riverqueue/river#702
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Dec 24, 2024
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. Fixes #249. [1] riverqueue/river#702
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Dec 24, 2024
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. Fixes #249. [1] riverqueue/river#702
bgentry
approved these changes
Dec 24, 2024
Contributor
bgentry
left a comment
There was a problem hiding this comment.
Nice! 
aside from the one commented code nit
Comment on lines
179
to
184
| func pgEnvConfigured() bool { | ||
| // Consider these two vars the bare minimum for an env-based configuration. | ||
| // return os.Getenv("PGHOST") != "" && os.Getenv("PGDATABASE") != "" | ||
| return os.Getenv("PGDATABASE") != "" | ||
| } |
Contributor
There was a problem hiding this comment.
Some commented code in here and a comment which only lines up with the commented variant (not the active one)
Contributor
Author
There was a problem hiding this comment.
Ah good catch. I changed my mind on this one. PGHOST defaults nicely to localhost so it seems okay to assume that PGDATABASE could be considered a bare minimum viable configuration.
Contributor
|
Oh, needs changelog too 🙏 |
64837de to
03b64d4
Compare
This one's related to #676, in which it was reported that database URLs work somewhat suboptimally when dealing with passwords that contain characters that traditionally require encoding in URLs (e.g. various types of brackets). I was originally going down a path wherein we'd offer an alternative that'd involve adding additional CLI parameters that could work as an alternative to `--database-url` based on these ones from psql: Connection options: -h, --host=HOSTNAME database server host or socket directory (default: "local socket") -p, --port=PORT database server port (default: "5432") -U, --username=USERNAME database user name (default: "brandur") -w, --no-password never prompt for password -W, --password force password prompt (should happen automatically) However, as I implemented it, I found that pgx somewhat surprisingly doesn't allow you to instantiate your own config struct. If not parsed from a URL, it requires you to assemble a more traditional connection string like `user=XX pass=YY database=ZZ` and parse that, which introduces its own special character encoding challenges when using something like a space, equal, or quotation mark. Digging further I realized that pgx automatically supports the whole set of PG* environmental variables like `PGHOST`, `PGPORT`, `PGUSER`, and supporting these would add a solid way of using the River CLI without a database URL using a common interface that's also supported by psql and a variety of other pieces of related software. Even better, since we expect most people to be using a database URL most of the time, env vars give us a way to provide an alternative, but one which doesn't add any complication to the existing CLI interface. In general fewer parameters is better because it keeps the help docs easy to understand. Another advantage is that this will technically let us support more complex connection setups involving things like TLS certificates. All of these vars are supported, and none of which had a great answer with a database URL: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` Fixes #676.
03b64d4 to
5b34e1c
Compare
Contributor
Author
|
Added changelog. |
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Jan 4, 2025
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. Fixes #249. [1] riverqueue/river#702
brandur
added a commit
to riverqueue/riverui
that referenced
this pull request
Jan 4, 2025
Accept the standard set of `PG*` env vars as an alternative to database configuration (e.g. `PGHOST`, `PGDATABASE`, etc.). This is mostly driven by having done something similar for the CLI in [1], but was also requested in #249. This turns out to be quite easy to do because pgx does all the heavy lifting. As noted in [1], a bonus of this is that it adds some additional configuration options that aren't very easily doable right now, for example around the use of an SSL certificate to connect to Postgres. We get automatic support for these vars: * `PGSSLCERT` * `PGSSLKEY` * `PGSSLROOTCERT` * `PGSSLPASSWORD` As part of this I also ended up rearranging some things in `main.go`. Not strongly married to this design, but the idea is to get it into a place where we can write tests for it, which previously wasn't possible. Fixes #249. [1] riverqueue/river#702
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one's related to #676, in which it was reported that database URLs
work somewhat suboptimally when dealing with passwords that contain
characters that traditionally require encoding in URLs (e.g. various
types of brackets).
I was originally going down a path wherein we'd offer an alternative
that'd involve adding additional CLI parameters that could work as an
alternative to
--database-urlbased on these ones from psql:However, as I implemented it, I found that pgx somewhat surprisingly
doesn't allow you to instantiate your own config struct. If not parsed
from a URL, it requires you to assemble a more traditional connection
string like
user=XX pass=YY database=ZZand parse that, whichintroduces its own special character encoding challenges when using
something like a space, equal, or quotation mark.
Digging further I realized that pgx automatically supports the whole
set of PG* environmental variables like
PGHOST,PGPORT,PGUSER,and supporting these would add a solid way of using the River CLI
without a database URL using a common interface that's also supported
by psql and a variety of other pieces of related software.
Even better, since we expect most people to be using a database URL most
of the time, env vars give us a way to provide an alternative, but one
which doesn't add any complication to the existing CLI interface. In
general fewer parameters is better because it keeps the help docs easy
to understand.
Another advantage is that this will technically let us support more
complex connection setups involving things like TLS certificates. All of
these vars are supported, and none of which had a great answer with a
database URL:
PGSSLCERTPGSSLKEYPGSSLROOTCERTPGSSLPASSWORDFixes #676.