(MODULES-1394) replace validate_db_connection type with custom type #879
Conversation
This replaces the defined type `validate_db_connection` with a new custom type called `postgresql_conn_validator`. It's functionality is nearly a copy of its predecessor with the exception of the `create_db_first` parameter which is taken care of with a collector in service.pp. The old type is still intact but all docs have been removed and a warning has been attached to it announcing its deprecation.
eputnam
force-pushed
the
validate_db_connection
branch
from
bfbce3a to
ac21c60
Compare
|
@hasegeli , it would be awesome to have your input on this |
manifests/validate_db_connection.pp
Outdated
| @@ -63,6 +65,9 @@ | |||
| $env = $pass_env | |||
| } | |||
|
|
|||
| warning($validate_cmd) | |||
| warning($env) | |||
|
|
|||
| validate do |value| | ||
| if value | ||
| value =~ /[0-9]+/ | ||
| end |
There was a problem hiding this comment.
Maybe it is better to check this as an integer.
| @resource = resource | ||
| end | ||
|
|
||
| def build_psql_cmd |
There was a problem hiding this comment.
Postgres has the PQping() command on its binary protocol to issue a connection check. It can be used with pg_isready binary. I think it would make a good default provider. It would be more efficient and easier to use, because you don't need to pass authentication. It also supports a custom timeout.
| cmd_parts = { | ||
| :host => "-h #{@resource[:host]}", | ||
| :port => "-p #{@resource[:port]}", | ||
| :db_username => "-U #{@resource[:db_username]}", |
There was a problem hiding this comment.
Some short and some long arguments don't please my eyes. I prefer --xxx=yyy format on scripts when possible to avoid argument parsing issues.
| end | ||
|
|
||
| def build_validate_cmd | ||
| "/bin/echo 'SELECT 1' | #{parse_connect_settings.join(' ')} #{build_psql_cmd} " |
There was a problem hiding this comment.
I wouldn't spawn another process but use --command='SELECT 1'.
ruby.rb
Outdated
| def psql_cmd | ||
| final_cmd = [] | ||
|
|
||
| cmd_init = "#{resource[:psql_path]} --tuples-only --quiet " |
There was a problem hiding this comment.
--no-psqlrc is also always a good idea on scripts. Otherwise the client's configuration can interfere.
eputnam
force-pushed
the
validate_db_connection
branch
from
e691732 to
8353538
Compare
Addresses a few of @hasegeli\'s comments. Adds a `command` parameter for more customizability.
eputnam
force-pushed
the
validate_db_connection
branch
from
8353538 to
61ad33d
Compare
(MODULES-1394) replace validate_db_connection type with custom type
This replaces the defined type
validate_db_connectionwith a newcustom type called
postgresql_conn_validator. It's functionality isnearly a copy of its predecessor with the exception of the
create_db_firstparameter which is taken care of with a collector inservice.pp. The old type is still intact but all docs have been removed
and a warning has been attached to it announcing its deprecation.