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

postgresql::server::extension needs to have defaults for postgresql_psql #582

Merged
merged 3 commits into from
Mar 19, 2015
Merged

postgresql::server::extension needs to have defaults for postgresql_psql #582

merged 3 commits into from
Mar 19, 2015

Conversation

rvstaveren
Copy link
Contributor

Using same pattern as with postgresql::server::database.
observed when installing the pg_trgm using pgsql 9.4 for puppetdb

affects: puppetlabs-postgresql 4.2.0

code used:

class { 'postgresql::globals':
manage_package_repo => true,
version => $postgres_version,
}->
class { 'postgresql::server':
listen_addresses => '*',
}->
class { '::puppetdb::database::postgresql':
manage_server => false,
database_name => $database_name,
database_username => $database_username,
database_password => $database_password,
before => [Class['puppetdb::server'],
Class['puppetdb::server::validate_db']],
}

class { 'postgresql::server::contrib':
}->
postgresql::server::extension { 'pg_trgm':
database => $database_name,
}

class { '::puppetdb::server':
listen_address => '0.0.0.0',
listen_port => '8080',
ssl_listen_address => '0.0.0.0',
ssl_listen_port => '8081',
open_ssl_listen_port => true,
database => 'postgres',
}

@igalic
Copy link
Contributor

igalic commented Mar 12, 2015

that.. makes sense.
do you know of other places where we're missing out on setting these defaults?

psql_group => $group,
psql_path => $psql_path,
port => $port,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you fix the indentation to 2 spaces?

@rvstaveren
Copy link
Contributor Author

@igalic it looks like manifests/server/initdb.pp uses postgresql_psql, but doesn't have a Postgresql_psql setting any defaults. I could do the same over there, as with the extension, but the question is whether a postgresql::server class is active too at the same time

@igalic
Copy link
Contributor

igalic commented Mar 13, 2015

this almost sounds like it's a call for… ACCEPTANCE TESTS!

@rvstaveren
Copy link
Contributor Author

Can't imagine that changes spaces in 6d937f0 broke the build :(

@igalic
Copy link
Contributor

igalic commented Mar 14, 2015

Failures:

1) postgresql::server service_ensure => running should set postgres password
Failure/Error: is_expected.to contain_exec('set_postgres_postgrespw').with({
expected that the catalogue would contain Exec[set_postgres_postgrespw] with unless set to "/usr/bin/psql -h localhost -c 'select 1' > /dev/null" but it is set to "/usr/bin/psql -h localhost -p 5432 -c 'select 1' > /dev/null"
# ./spec/unit/classes/server_spec.rb:42:in `block (3 levels) in <top (required)>'

2) Puppet::Type::Postgresql_psql::ProviderRuby with port string executes with the given port
Failure/Error: provider.run_sql_command("SELECT something")
Postgresql_psql[rspec psql test](provider=ruby) received :run_command with unexpected arguments
expected: (["psql", "-p", "5555", "-t", "-c", "SELECT something"], "postgres", "postgres")
got: (["psql", "-p", "5555", "-t", "-c", "\"SELECT something\""], "postgres", "postgres")
# ./lib/puppet/provider/postgresql_psql/ruby.rb:21:in `block in run_sql_command'
# ./lib/puppet/provider/postgresql_psql/ruby.rb:20:in `chdir'
# ./lib/puppet/provider/postgresql_psql/ruby.rb:20:in `run_sql_command'
# ./spec/unit/puppet/provider/postgresql_psql/ruby_spec.rb:79:in `block (3 levels) in <top (required)>'

these are the failures as reported by travis

@igalic
Copy link
Contributor

igalic commented Mar 14, 2015

you might have to rebase, post #584

@rvstaveren
Copy link
Contributor Author

@igalic thx for the pointer! btw, do you want to have the manifests/server/initdb.pp case addressed too in the same pull request?

@igalic
Copy link
Contributor

igalic commented Mar 15, 2015

if it's not too much effort for you: sure!
but please change the commit messages and the title to say you're addressing a general case.

also, once done you may wanna clean up these commits by squashing them down to a single one (or… few:)

@igalic
Copy link
Contributor

igalic commented Mar 15, 2015

i just noticed your rebase is actually a merge commit, please take a look at this

hunner added a commit that referenced this pull request Mar 19, 2015
…ql_psql-fix

postgresql::server::extension needs to have defaults for postgresql_psql
@hunner hunner merged commit 7018a5f into puppetlabs:master Mar 19, 2015
@rvstaveren rvstaveren deleted the rvstaveren-extension-postgresql_psql-fix branch March 19, 2015 17:48
cegeka-jenkins pushed a commit to cegeka/puppet-postgresql that referenced this pull request Oct 23, 2017
…n-postgresql_psql-fix

postgresql::server::extension needs to have defaults for postgresql_psql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants