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

Add attributes to configure Consul ports #26

Merged
merged 1 commit into from
Nov 15, 2014
Merged

Add attributes to configure Consul ports #26

merged 1 commit into from
Nov 15, 2014

Conversation

jubianchi
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f1e4da4 on jubianchi:consul-ports into 3548309 on johnbellone:master.

@johnbellone
Copy link
Contributor

@reset Any thoughts?

@reset
Copy link
Contributor

reset commented Sep 17, 2014

@johnbellone this accomplishes the goal but the overall approach we're using to configure the service isn't very safe in the first place. I'd say this is safe to merge in but we should look at validating our attributes to raise awareness at the start of a chef run, not in the middle of it, of misconfigured attributes.

@johnbellone johnbellone modified the milestones: 0.5.0, 0.3.1 Oct 15, 2014
@johnbellone
Copy link
Contributor

@reset What's the best way to verify the attributes?

@reset
Copy link
Contributor

reset commented Oct 29, 2014

@johnbellone unfortunately the attributes you define in the metadata of the cookbook don't actually enforce anything... that would be the ideal place. So instead I just write Ruby modules in a Library that take the node object and perform validation on all the things that I expect a node to have.

Honestly we (as a community) need to help improve this and make cookbook metadata define the rules and then have them run at convergence time.

@johnbellone
Copy link
Contributor

@jubianchi Can you rebase?

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.95%) when pulling b8bfa3d on jubianchi:consul-ports into 3643ab6 on johnbellone:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0274688 on jubianchi:consul-ports into 3643ab6 on johnbellone:master.

@jubianchi
Copy link
Contributor Author

@johnbellone done!

johnbellone added a commit that referenced this pull request Nov 15, 2014
Add attributes to configure Consul ports
@johnbellone johnbellone merged commit 92a11df into sous-chefs:master Nov 15, 2014
@jubianchi
Copy link
Contributor Author

👍

@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants