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

Make database validation optional #157

Merged
merged 1 commit into from
Jan 12, 2015

Conversation

robinbowes
Copy link
Contributor

We don't want to validate the DB connection when building puppetdb nodes as we're building AMIs on AWS that won't necessarily have access to the RDS instance they will eventually use when instantiated.

This PR adds a couple of flags to make the db validation optional, one for database, one for read_database.

R.

@robinbowes
Copy link
Contributor Author

It seems the Travis CI failures are only on Ruby 1.8.7. and not related to my changes.

}

$ini_setting_require = str2bool($database_validate) ? {
false => undef,
default => Class['puppetdb::server::validate_db'],
Copy link
Contributor

Choose a reason for hiding this comment

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

You could avoid this entirely by setting the class's (puppetdb::server::validate_db) before param to Ini_setting <| |> ; collecting on all Ini_setting's :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not worth doing it this time around, but worth thinking about later. If we ever get around to implementing a defined resource to wrap these ini settings or something, probably better to do it using that resource instead of inifile, since inifile could bleed into other modules and create more edges than necessary in the graph. Extraneous edges == bad sometimes, because it makes for a larger catalog.

@kbarber
Copy link
Contributor

kbarber commented Jan 8, 2015

@robinbowes ping :-).

@kbarber
Copy link
Contributor

kbarber commented Jan 12, 2015

@robinbowes see previous comments, also I think this needs a rebase now.

@robinbowes
Copy link
Contributor Author

Rebased.

@kbarber
Copy link
Contributor

kbarber commented Jan 12, 2015

@robinbowes this looks okay now, one thing we forgot to ask for before was the documentation changes for this new parameter in the README.md, can you add this? Sorry, shouldn't have noticed this earlier.

Also - you might want to squash your commits into 1 while you're at it.

@robinbowes
Copy link
Contributor Author

How's that?

@kbarber
Copy link
Contributor

kbarber commented Jan 12, 2015

@robinbowes perfect, thanks mate :-).

kbarber added a commit that referenced this pull request Jan 12, 2015
@kbarber kbarber merged commit d4de12f into puppetlabs:master Jan 12, 2015
@robinbowes robinbowes deleted the make_validate_db_optional branch January 12, 2015 16:56
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.

5 participants