Pass all PG settings as env vars #990
Conversation
| def pg_sql_statement(config, statement) | ||
| pg_command_base(config, 'psql', "-d #{config[:database]} -t -c \"#{statement}\"") | ||
| def pg_sql_statement(statement) | ||
| "psql -t -c \"#{statement}\"" |
There was a problem hiding this comment.
I'd be tempted to pass in the statement via stdin but wonder if that works well with runuser (if we do that)
There was a problem hiding this comment.
runuser is only used for local sql, and there we don't need statements.
There was a problem hiding this comment.
that said, our execute helpers don't do stdin right now, so it'd need more refactoring.
| "PGPASSWORD='#{config[:password]}' #{command} -U #{config[:username]} -h #{config[:host]} -p #{config[:port]} #{args}" | ||
| def pg_env(config) | ||
| { | ||
| 'PGHOST' => config.fetch(:host, 'localhost'), |
There was a problem hiding this comment.
Do we need a default or can we trust psql to figure it out?
There was a problem hiding this comment.
No idea, copied from your fm patch ;)
There was a problem hiding this comment.
There I had it because local? compares to localhost and I didn't want to change that. Perhaps we should make it more robust so it can parse any postgresql url. I'd like to be able to use unix sockets and ident auth but now we'd break foreman-maintain
There was a problem hiding this comment.
Here we have remote_host?(hostname) which does something very similar.
The old code just assumed config[:host] is set (the command would be wrong if it were unset), which we can do here too?
There was a problem hiding this comment.
I think keeping a default for localhost is OK for now then.
Yeah, but our helpers incorporate the escaping and I like helpers ;) |
|
You can pass an array and avoid any escaping |
evgeni
force-pushed
the
pgenv
branch
2 times, most recently
from
c4d76d4 to
0447a0d
Compare
I meant the "escaping from Puppet ruby env". My point is:
So if we can drop the "puppet-env-escaping" all together, cool (but I think we can't, as we call f-m and f-rake, which need system ruby), but scope-creep. Here I just want to pass in PG things via env :) |
|
/packit build |
| expect(context).to have_received(:'execute!').with("runuser -l postgres -c 'PGSETUP_INITDB_OPTIONS=\"--lc-collate=en_US.UTF-8 --lc-ctype=en_US.UTF-8 --locale=en_US.UTF-8\" postgresql-setup --upgrade'", false, true) | ||
| expect(context).to have_received(:'execute!').with("runuser -l postgres -c 'PGSETUP_INITDB_OPTIONS=\"--lc-collate=en_US.UTF-8 --lc-ctype=en_US.UTF-8 --locale=en_US.UTF-8\" postgresql-setup --upgrade'", false, true, {}) |
There was a problem hiding this comment.
runuser -l clears the environment, so that's why I did not use the env stuff here too.
|
@ekohl done |
needs theforeman/kafo#378