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

What's the correct way to set config options now? #978

Closed
chriskuehl opened this issue Nov 24, 2016 · 4 comments
Closed

What's the correct way to set config options now? #978

chriskuehl opened this issue Nov 24, 2016 · 4 comments

Comments

@chriskuehl
Copy link

First, thanks a lot for maintaining this module!

I'd like to ask about the current best practice for declaring parameters. We're using the latest (0.5.0) version of this module. Currently, we're doing something like this:

class { 'nginx':
  package_name => 'nginx-extras',
  manage_repo  => false,
  confd_purge  => true,
  vhost_purge  => true,
}

This works okay but we always get a warning:

Nov 23 22:10:05 whirlwind puppet-agent[5843]: [nginx] *** DEPRECATION WARNING***: HI! I notice that you're declaring some attributes in Class[nginx]. It is highly recommended to set these values via Hiera going forward. This will become mandatory in the near future. Please check out https://github.com/voxpupuli/puppet-nginx/blob/master/docs/hiera.md for more information.

Additionally, we can only declare this class once, which is sometimes inconvenient (even though we always use the same parameters).

The link no longer works, but for reference, here's an older version: https://github.com/voxpupuli/puppet-nginx/blob/163556f769eb39e33b2592a504c9da2634b52b32/docs/hiera.md

It looks like there's been a recent change to the way these config parameters are set in #950 and I'm having trouble determining the recommended best practice for setting these now. Originally I tried following the advice from the now-deleted hiera.md docs:

nginx::config::package_name: nginx-extras
nginx::config::manage_repo: false
nginx::config::confd_purge: true
nginx::config::vhost_purge: true

...but some of these parameters didn't work (manage_repo and package_name), which makes sense looking at https://github.com/voxpupuli/puppet-nginx/blob/621778d4b5c2f9dffa4aa1bffc8db317a4cb63b8/manifests/config.pp.

From looking through the code, it looks like we can set these on nginx::package instead, but that didn't work either, so I think I might be approaching this wrong.

I also noticed in #950 that it was mentioned that the warning was being removed, but I'm not sure whether that means it's suggested to declare parameters on the class directly or not.

Could you clarify what the recommended best practice for configuring these settings is now? Thanks!

@wyardley
Copy link
Collaborator

Hi @chriskuehl @jvperrin
Sorry for the late reply.

Yes, things are in a bit of a state of transition, the next release will get rid of the public ::nginx::config namespace, so all parameters will be set at ::nginx scope.

As for earlier versions, including current Forge release, most of the parameters that relate to the module itself are in ::nginx:: scope (probably why manage_repo is there, for example), but basically all other params are in ::nginx::config. As of now, looking at the code is the only way to really tell for sure, but we'll try to cut a release soon.

@saz
Copy link
Sponsor Contributor

saz commented Nov 30, 2016

Is there an ETA for the next release?

@wyardley
Copy link
Collaborator

@saz probably waiting on #980 (should go through soon), which will be another pretty sizable change from a user's perspective, also may wait on #938, but I think we'll be trying to cut a release "soon", as in 1-3 weeks, give or take?

@wyardley
Copy link
Collaborator

wyardley commented Dec 5, 2016

We're hoping to actually just cut a long-overdue 1.x. However, may need to do one or two additional fixes before we release it. Hope that answers everyone's questions; I'm going to close this one now, as I think the original question has been answered.

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

No branches or pull requests

3 participants