Skip to content

(MODULES-1394) replace validate_db_connection type with custom type #879

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

Merged
merged 3 commits into from
Jun 12, 2017

Conversation

eputnam
Copy link
Contributor

@eputnam eputnam commented Jun 8, 2017

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 added 2 commits June 8, 2017 14:48
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 eputnam force-pushed the validate_db_connection branch from bfbce3a to ac21c60 Compare June 8, 2017 21:49
@eputnam
Copy link
Contributor Author

eputnam commented Jun 8, 2017

@hasegeli , it would be awesome to have your input on this

Copy link
Contributor

@hasegeli hasegeli left a comment

Choose a reason for hiding this comment

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

Looks good to me in general.

@@ -63,6 +65,9 @@
$env = $pass_env
}

warning($validate_cmd)
warning($env)

Copy link
Contributor

Choose a reason for hiding this comment

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

Debugging?

validate do |value|
if value
value =~ /[0-9]+/
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it is better to check this as an integer.

@resource = resource
end

def build_psql_cmd
Copy link
Contributor

Choose a reason for hiding this comment

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

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]}",
Copy link
Contributor

Choose a reason for hiding this comment

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

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} "
Copy link
Contributor

Choose a reason for hiding this comment

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

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 "
Copy link
Contributor

Choose a reason for hiding this comment

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

--no-psqlrc is also always a good idea on scripts. Otherwise the client's configuration can interfere.

@eputnam eputnam force-pushed the validate_db_connection branch from e691732 to 8353538 Compare June 9, 2017 15:55
Addresses a few of @hasegeli\'s comments. Adds a `command` parameter for more customizability.
@eputnam eputnam force-pushed the validate_db_connection branch from 8353538 to 61ad33d Compare June 9, 2017 15:56
@HelenCampbell HelenCampbell merged commit 4dac726 into puppetlabs:master Jun 12, 2017
@eputnam eputnam deleted the validate_db_connection branch June 27, 2017 21:14
cegeka-jenkins pushed a commit to cegeka/puppet-postgresql that referenced this pull request Feb 3, 2022
(MODULES-1394) replace validate_db_connection type with custom type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants