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

Don't include_recipe passenger in repo recipe #390

Closed
wants to merge 1 commit into from
Closed

Don't include_recipe passenger in repo recipe #390

wants to merge 1 commit into from

Conversation

donaldguy
Copy link

@donaldguy donaldguy commented Dec 21, 2015

Doing this include at this point, before nginx package is installed, breaks default or package recipe on first run if using passenger repos (because config dir doesn't yet exist when writing passenger.conf). It does mean you have to include_recipe 'nginx::passenger' later yourself.

This doesn't seem unreasonable, but would account to a breaking change for people who were depending on implicit inclusion (though the packages and config files are likely already in place for them)

A safer version of this would do the include at this point if nginx already installed but defer it to end of package recipe if first-install

Another alternative might be to switch the writing of the config to a delayed notification off ... something

A super ugly workaround with the current cookbook for an initial install can be achieved as follows:

ignore_further_includes = run_context.send(:loaded_recipes_hash)
ignore_further_includes['nginx::passenger'] = true
include_recipe 'nginx'
ignore_further_includes.delete('nginx::passenger')

This change is Reviewable

@aerodynamik
Copy link

+1 for this change

@tas50
Copy link
Contributor

tas50 commented Sep 17, 2017

Thanks for opening this PR. With the merge of the chef_nginx codebase back into the nginx cookbook I believe it’s no longer necessary. I’m going to close this out now, and I hope you can check out the massive changes that landed on master here #435

@tas50 tas50 closed this Sep 17, 2017
@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.

None yet

4 participants