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

Default recipe for attribute-driven configuration #100

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bodgix
Copy link

@bodgix bodgix commented Dec 20, 2018

This commit adds the default recipe which creates
hosts file entries based on a node attribute.

This new functionality can simplify the use of
the hosts file cookbook in some cases because
it makes managing the hosts file possible with
just data.

Issue: #85

bodgix and others added 3 commits July 13, 2017 16:45
This commit adds the default recipe which creates
hosts file entries based on a node attribute.

This new functionality can simplify the use of
the hosts file cookbook in some cases because
it makes managing the hosts file possible with
just data.
we're not meteorologists
@tas50
Copy link
Contributor

tas50 commented Dec 20, 2018

This seems like a real anti-pattern since this cookbook provides a resource and should be used as a library cookbook.

@bodgix
Copy link
Author

bodgix commented Dec 20, 2018

In my company, we don't have applications that need to manage /etc/hosts. We just need to manage it for a host.

If configuring /etc/hosts was a part of some bigger thing, like for example, cookbook_my_wordpress_blog needed to manage /etc/hosts for the wordpress app, we would call resources provided by hostsfile from there.

But since it's not the case, and in our case there isn't any reasonable place we could call the resources from we decided it made more sense to add a default recipe instead of creating a cookbook for example mycompany_hostsfile whose only purpose would be to define one resource.

@bodgix
Copy link
Author

bodgix commented Dec 21, 2018

@bby-bishopclark I gave it another go and fixed the CI warning which I knew how.

This CI error:

libraries/entry.rb:129:5: W: Lint/DuplicateMethods: Method HostsFile::Entry#priority= is defined at both libraries/entry.rb:94 and libraries/entry.rb:129.
    def priority=(new_priority)
    ^^^

Is not related to my change. Must have been there before.

Not sure if this change gets merged anyway. We've been using this version at my company but we do not require it is merged into upstream because we don't depend on the supermarket so we're happy with continuing to use a fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants