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

Support for remote puppetdb #41

Merged
merged 1 commit into from
Apr 12, 2013

Conversation

fhrbek
Copy link
Contributor

@fhrbek fhrbek commented Mar 18, 2013

This PR contains changes that were required to handle connection validation in a more flexible way. The new solution also allows (on demand, not by default) to store configuration settings even when puppetdb is not reachable.

@@ -1,12 +1,11 @@
name 'puppetlabs-puppetdb'
version '1.1.5'
source 'git://github.com/puppetlabs-puppet/puppetlabs-puppetdb.git'
version '1.1.6'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you not increment this? A release increment is decided at release time, its not tied to a particular patch. The next release may be 1.2.0 for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, does it mean that 1.2.0 a good target version for me now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just leave it as it was '1.1.5'. The future version should be determined in a release patch not in a particular feature patch like this one.

@fhrbek
Copy link
Contributor Author

fhrbek commented Mar 19, 2013

Thanks for your comments, I've been working on fixing the issues.

@kbarber
Copy link
Contributor

kbarber commented Mar 20, 2013

@fhrbek cheers - added some more comments to your comments :-).

@puppetcla
Copy link

CLA Signed by fhrbek on 2013-01-07 21:00:00 -0800

@fhrbek
Copy link
Contributor Author

fhrbek commented Apr 12, 2013

I added an rpec test (is it what you meant?) and I also removed the option (that I had introduced) of reading the puppetdb.conf file which is not needed any more after changing the pe_puppetdb module.

It would be great if we could finally merge this PR and #37.

@kbarber
Copy link
Contributor

kbarber commented Apr 12, 2013

@fhrbek. This will need to be rebased as well

@fhrbek
Copy link
Contributor Author

fhrbek commented Apr 12, 2013

I've rebased. We've both added spec tests. They don't seem to be in conflict but most of yours fail on my machine, for example

1) puppetdb on a supported platform when using default values for puppetdb class 
     Failure/Error: it { should contain_class('puppetdb') }
     Puppet::Error:
       Could not find class puppetdb for filiphrbeknb.cloudsmith on node filiphrbeknb.cloudsmith
     # ./spec/unit/classes/init_spec.rb:16:in `block (4 levels) in <top (required)>'

Am I doing anything wrong?

@kbarber
Copy link
Contributor

kbarber commented Apr 12, 2013

@fhrbek are you running 'rake spec' to run the tests?

kbarber added a commit that referenced this pull request Apr 12, 2013
@kbarber kbarber merged commit fc5cdd4 into puppetlabs:master Apr 12, 2013
@fhrbek fhrbek deleted the conn_validator_refactoring branch April 12, 2013 13:25
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.

4 participants