Skip to content
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

Read Postgres settings from environment variables #2541

Merged
merged 4 commits into from
Feb 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ For more information about this file see also [Keep a Changelog](http://keepacha
- Models monitoring container for Docker now shows a webpage with models it has seen
- Added small container to check if certain services are up, used as initi container for kubernetes
- Documentation how to run ED using singularity
- PEcAn.DB gains new function `get_postgres_envvars`, which tries to look up connection parameters from Postgres environment variables (if they are set) and return them as a list ready to be passed to `db.open`. It should be especially useful when writing tests that need to run on systems with many different database configurations (#2541).

## [1.7.1] - 2018-09-12

Expand Down
1 change: 1 addition & 0 deletions base/db/NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export(fancy_scientific)
export(get.id)
export(get.trait.data)
export(get.trait.data.pft)
export(get_postgres_envvars)
export(get_run_ids)
export(get_users)
export(get_var_names)
Expand Down
75 changes: 75 additions & 0 deletions base/db/R/get_postgres_envvars.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
#' Look up Postgres connection parameters from environment variables
#'
#' Retrieves database connection parameters stored in any of the
#' environment variables known by Postgres,
#' using defaults from `...` for parameters not set in the environment.
#' In a standard PEcAn installation only a few of these parameters
#' will ever be set, but we check all of them anyway in case you need to do
#' anything unusual.
#'
#' The list of environment variables we check is taken from the
#' [Postgres 12 manual](https://postgresql.org/docs/12/libpq-envars.html),
#' but it should apply to older Postgres versions as well.
#' Note that this function only looks for environment variables that control
#' connection parameters; it does not retrieve any of the variables related to
#' per-session behavior (e.g. PGTZ, PGSYSCONFDIR).
#'
#' @param ... defaults for parameters not found in the environment,
#' in `name = value` form
#' @return list of connection parameters suitable for passing on to `db.open`
#'
#' @examples
#' host <- Sys.getenv("PGHOST") # to restore environment after demo
#'
#' Sys.unsetenv("PGHOST")
#' get_postgres_envvars()$host # NULL
#' get_postgres_envvars(host = "default", port = 5432)$host # "default"

#' # defaults are ignored for a variable that exists
#' Sys.setenv(PGHOST = "localhost")
#' get_postgres_envvars()$host # "localhost"
#' get_postgres_envvars(host = "postgres")$host # still "localhost"
#'
#' # To override a set variable, edit the returned list before using it
#' con_parms <- get_postgres_envvars()
#' con_parms$host # "localhost"
#' con_parms$host <- "postgres"
#' # db.open(con_parms)
#'
#' Sys.setenv(PGHOST = host)
#' @export
get_postgres_envvars <- function(...) {
pg_vars <- list(
host = "PGHOST",
hostaddr = "PGHOSTADDR",
port = "PGPORT",
dbname = "PGDATABASE",
user = "PGUSER",
password = "PGPASSWORD",
passfile = "PGPASSFILE",
service = "PGSERVICE",
options = "PGOPTIONS",
application_name = "PGAPPNAME",
ssl_mode = "PGSSLMODE",
requiressl = "PGREQUIRESSL",
sslcompression = "PGSSLCOMPRESSION",
sslcert = "PGSSLCERT",
sslkey = "PGSSLKEY",
sslrootcert = "PGSSLROOTCERT",
sslcrl = "PGSSLCRL",
requirepeer = "PGREQUIREPEER",
krbsrvname = "PGKRBSRVNAME",
gsslib = "PGGSSLIB",
connect_timeout = "PGCONNECT_TIMEOUT",
client_encoding = "PGCLIENTENCODING",
target_session_attrs = "PGTARGETSESSIONATTRS")

vals <- Sys.getenv(pg_vars)
names(vals) <- names(pg_vars)
vals <- vals[vals != ""]

defaults <- list(...)
defaults <- defaults[!(names(defaults) %in% names(vals))]

append(vals, defaults)
}
50 changes: 50 additions & 0 deletions base/db/man/get_postgres_envvars.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion base/db/tests/testthat.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ library(testthat)

library(PEcAn.DB)
library(RPostgreSQL)
dbparms <- list(host = "localhost", driver = "PostgreSQL", user = "bety", dbname = "bety", password = "bety")
dbparms <- get_postgres_envvars(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

host = "localhost",
driver = "PostgreSQL",
user = "bety",
dbname = "bety",
password = "bety")

if(db.exists(dbparms)){
con <- db.open(dbparms)
Expand Down
23 changes: 18 additions & 5 deletions base/db/tests/testthat/helper-db-setup.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,35 @@ get_db_params <- function() {
# host = "localhost",
# port = 5432))
# ```
# OR by setting Postgres environment parameters in your shell:
# ```
# export PGHOST=localhost
# export PGUSER=bety
# [etc]
# ```
option_params <- getOption("pecan.db.params")
# Check if running on continuous integration (CI)
# If yes, skip this test
is_ci <- Sys.getenv('CI') != ''
is_ci <- Sys.getenv("CI") != ""
if (!is.null(option_params)) {
return(option_params)
} else if (is_ci) {
return(list(host = "localhost", user = "bety", password = "bety",
driver = "Postgres"))
return(get_postgres_envvars(
host = "localhost",
user = "bety",
password = "bety",
driver = "Postgres"))
} else {
if (PEcAn.remote::fqdn() == "pecan2.bu.edu") {
return(list(host = "psql-pecan.bu.edu", driver = "PostgreSQL",
dbname = "bety", user = "bety", password = "bety"))
} else {
return(list(host = "localhost", driver = "PostgreSQL",
user = "bety", dbname = "bety", password = "bety"))
return(get_postgres_envvars(
host = "localhost",
driver = "PostgreSQL",
user = "bety",
dbname = "bety",
password = "bety"))
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions base/settings/tests/testthat/helper-get.test.settings.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@
},
silent = TRUE)

# NB environment variables override values in XML here!
# This is opposite of usual PEcAn rule that XML values always win,
# but useful here because it allows testing on systems where we
# don't know the database configuration in advance
settings$database$bety <- do.call(
PEcAn.DB::get_postgres_envvars,
settings$database$bety)

if (is.null(settings)) {
skip("Can't get a valid test Settings right now. Skipping test. ")
}
Expand Down
3 changes: 3 additions & 0 deletions models/biocro/tests/testthat/test-run.biocro.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ ref_mat <- mean(ref_met$Temp)
# run setup
metpath <- "data/US-Bo1"
settings <- PEcAn.settings::read.settings("data/pecan.biocro.xml")
settings$database$bety <- do.call(
PEcAn.DB::get_postgres_envvars,
settings$database$bety)
config <- PEcAn.settings::prepare.settings(settings)
config$pft$type$genus <- "Salix"
config$run$start.date <- as.POSIXct("2004-01-01")
Expand Down
3 changes: 3 additions & 0 deletions models/biocro/tests/testthat/test.model2netcdf.BIOCRO.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ dir.create(outdir, showWarnings = FALSE, recursive = TRUE)
file.copy(from = "data/result.RData", to = outdir)

settings <- PEcAn.settings::read.settings("data/pecan.biocro.xml")
settings$database$bety <- do.call(
PEcAn.DB::get_postgres_envvars,
settings$database$bety)

start_date <- settings$run$start.date

Expand Down
3 changes: 3 additions & 0 deletions models/biocro/tests/testthat/test.write.configs.BIOCRO.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ context("checking write.configs.BIOCRO")

settings.xml <- file.path("data", "pecan.biocro.xml")
settings <- PEcAn.settings::read.settings(settings.xml)
settings$database$bety <- do.call(
PEcAn.DB::get_postgres_envvars,
settings$database$bety)

testthat::skip_if_not(PEcAn.DB::db.exists(settings[[c("database", "bety")]]))

Expand Down
6 changes: 4 additions & 2 deletions modules/benchmark/tests/testthat/test-align_pft.R
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@

context("align_pft")

con <- PEcAn.DB::db.open(
list(host = "localhost", user = "bety", password = "bety"))
con <- PEcAn.DB::db.open(PEcAn.DB::get_postgres_envvars(
host = "localhost",
user = "bety",
password = "bety"))
teardown(PEcAn.DB::db.close(con))

observation_one <- c("AMCA3", "AMCA3", "AMCA3", "AMCA3")
Expand Down
15 changes: 10 additions & 5 deletions modules/data.land/tests/testthat/test.PFT_consistency.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@ library(PEcAn.settings)
library(PEcAn.DB)
library(RPostgreSQL)

betyparms <- list(host = 'localhost', dbname = 'bety',
user = 'bety', password = 'bety', driver = 'PostgreSQL', write = FALSE)
fiaparms <- list(host = 'localhost', dbname = 'fia5data',
user = 'bety', password = 'bety', driver = 'PostgreSQL')
if(db.exists(params = betyparms) & db.exists(fiaparms)){
betyparms <- PEcAn.DB::get_postgres_envvars(
host = "localhost",
dbname = "bety",
user = "bety",
password = "bety",
driver = "PostgreSQL",
write = FALSE)
fiaparms <- betyparms
fiaparms$dbname <- "fia5data"
if (db.exists(params = betyparms) & db.exists(fiaparms)) {


context("Testing consistency of FIA PFTs")
Expand Down
7 changes: 6 additions & 1 deletion scripts/add.util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@ if [ -z "$FQDN" ]; then
FQDN=$( hostname -f )
fi

# postgres hostname
if [ -z "$PGHOST" ]; then
PGHOST="localhost"
fi

# command to execute to add items to BETY database
if [ -z "$PSQL" ]; then
PSQL="psql -h localhost -U bety bety -q -t -c"
PSQL="psql -h ${PGHOST} -U bety bety -q -t -c"
fi

# folder to data, this is assumed to be installed at the same level
Expand Down