Read Postgres settings from environment variables

[infotroph]

Description

• New function PEcAn.DB::get_postgres_envvars: takes a list of default Postgres connection parameters, returns a list updated with the values from as many of the environment variables recognized by Postgres as are defined in your environment. In practice I doubt you'll be setting many more than PGHOST, PGPORT and maybe PGUSER, but for completeness I implemented all variables that control connection parameters.

• Update all packages' tests to use get_postgres_envvars where they were previously hardcoding host=localhost

Motivation and Context

Lots of the settings XML files in tests/* and <pkgname>/tests/testthat hard-code settings$database$bety$host as "localhost", and various other places in the code assume that "localhost" is a reasonable default when the hostname isn't specified. This has been biting me regularly when I try to run package tests inside the Docker stack, where we need host = "postgres".

In the long run, lots of these tests ought to be rewritten to avoid needing database access at all--they're supposed to be unit tests, and most units don't touch the database themselves!--but in the meantime we need a way to run the test suites we have with the database configurations that exist in the wild.

What I've built here is sort of a shim function: It looks for environment variables that may or may not be set, and if they are set it uses them to override the values it was given as input. The result is that you don't need a separate settings file for every possible database configuration; simply read one file and pass its database$bety section through get_postgres_envvars at runtime (after setting the relevant env vars, of course).

The big ol' downside: This function flagrantly breaks PEcAn's usual rule that the values in settings.xml always win over any conflicting defaults, and it does it in a way (environment variables) that's famously nontransparent and hard to debug unless you know what you're looking for. I expect we will not want to use this function very much outside of tests, but for tests I don't see any other good options for balancing reusable settings files against machine-specific database configurations -- I'm open to alternate proposals if anyone has them.

Review Time Estimate

• Immediately
• Within one week
• When possible

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the CHANGELOG.md.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[robkooper]

one extra comment, but good to merge.

[robkooper]

would it make sense to have most of these coded in get_postgres_envvars as defaults?

[infotroph]

I wondered that too, and could be persuaded either way. At the moment I leaning toward thinking it's less magic without, because then what you get back is strictly what you put in modulo whatever PG* vars are set. Thoughts?

[robkooper]

roll a 🎲 and see what happens. I can be convinced of either solution, hence I approved the pull request.