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 extra_config_section to take random configuration #31

Closed
dshepherd-descartes opened this issue Nov 7, 2016 · 15 comments
Closed

Comments

@dshepherd-descartes
Copy link

extra_config_section only takes a hash of parameters. That means you cannot configure repeating keys within the same extra_config_section, even if that would make sense.

It would be nice if it could read a list as well as a hash. This could be a list of hashes for convenience of parsing.

@dshepherd-descartes
Copy link
Author

Here is a patch against 0.3.0 that performs the changes necessary to support importing a list of hashes (and retains the old behaviour for hashes as well). I hope it works for you.

puppet-squid-extra-config.diff.txt

@traylenator
Copy link
Contributor

@dshepherd-descartes thanks for the patch, any chance you could submit as a MR?

@jadestorm
Copy link

I'd be happy to turn this into a PR on your behalf @dshepherd-descartes if you don't really want to deal with it -- that said you'd lose proper credit for it in github if I did that. =/

@ralfbosz
Copy link
Contributor

ralfbosz commented Jan 16, 2017

could somebody implement this? this would help adding things like refresh_pattern, always_direct & never_direct.

@jadestorm
Copy link

I'll go ahead and make a pull request based off the patch and we'll go from there.

@jadestorm
Copy link

#44

@ralfbosz
Copy link
Contributor

ralfbosz commented Jan 17, 2017

I've been thinking about this today, but isn't it easier to just 'change' the squid.conf.extra_config_section.erb:

# <%= @comment %>
<% @config_entries.each do |k,v| -%>
<% if v.is_a?(Array) -%>
<% v.each do |v2| %>
<%= k %> <%= v2 %>
<% end -%>
<% else -%>
<%= k %> <%= v %>
<% end -%>
<% end -%>

I don't see the point of why the array in the extra_config_section should be put on one line with a space between. I'm busy making a push-request for this, but haven't had time yet to push it.

@jadestorm
Copy link

Probably =) I spent a grand total of 1 minute on this (literally just applied the patch) so if you have a better way to take care of it be my guest! Would you like me to retract me PR or is your PR a PR to .. my .. PR.. fork.. whatever. =)

@ralfbosz
Copy link
Contributor

Since your PR is failing, the chance of it getting merges is unlikely.
I will post my PR tomorrow, hopefully with a working test, the PR is based on the "master", not on any new PR's...

@jadestorm
Copy link

Ok since you seem more enthused about pursuing this than I, I'll retract mine. =)

@ralfbosz
Copy link
Contributor

I've used the diff from @dshepherd-descartes for the manifest/extra_config_section.pp, the template-file was adjusted. Added an example in the readme and adjusted the rspec.

#45 is here !

ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Jan 20, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to add an option on multiple lines instead
of one big line.

This commit addresses issue voxpupuli#31
ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Jan 20, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to create multiple lines instead of one
big line with all the parameters.

This commit addresses issue voxpupuli#31
@cedef
Copy link
Contributor

cedef commented Apr 18, 2017

Any news when #45 will be merged into master ? Is there anything blocking the merge ?

ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Apr 26, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to create multiple lines instead of one
big line with all the parameters.

This commit addresses issue voxpupuli#31
ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Apr 26, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to create multiple lines instead of one
big line with all the parameters.

This commit addresses issue voxpupuli#31
ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Apr 26, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to create multiple lines instead of one
big line with all the parameters.

This commit addresses issue voxpupuli#31
ralfbosz added a commit to ralfbosz/puppet-squid that referenced this issue Jun 30, 2017
The option "extra_config_section" can be defined as an array,
this adds the option to create multiple lines instead of one
big line with all the parameters.

This commit addresses issue voxpupuli#31
@ralfbosz
Copy link
Contributor

It's merged !!!

@jadestorm
Copy link

@ralfbosz Awesome news! Just wrapped back around to checking on some of the outstanding things I had going. =) Thanks for taking care of this!

@ekohl
Copy link
Member

ekohl commented Nov 16, 2017

Given it's merged I'm going to close this.

@ekohl ekohl closed this as completed Nov 16, 2017
@traylenator traylenator changed the title Minor enhancement request Support extra_config_section to take random configuration Mar 27, 2018
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

6 participants