change listen attributes to be hash of addresses and ports to listen to #378
Conversation
|
FWIW, the travis failures were failing before my PR. |
|
I would love to see this made backwards-compatible, checking for the old attribute if it exists & using the older behavior, and emitting a warning that the user should upgrade. |
|
I believe the changes I made to the It doesn't give a message about them being deprecated or anything like that though. |
|
@martinb3, is there some preferred place to have it give a warning about deprecated attributes? I'm guessing that the right place would not be inside the |
|
I had been doing the conversion of |
This should fix the issue that was going on w/ PR sous-chefs#372 The attributes `node['apache']['listen']` will be a hash indexed by address where the values are ports to listen to. For example: ```json { "apache": { "listen": { "*": ["80", "443"], "127.0.0.1": [ "12345" ] } } } ``` would create this ports.conf: ``` Listen *:80 Listen *:443 Listen 127.0.0.1:12345 ``` Which was impossible with the older listen_ports and listen_addresses attributes.
….conf.erb and print a deprecation warning.
|
Realized I needed to use a |
|
Build failing because of berkshelf nonsense. @svanzoest, @drpebcak, looks like it might be time to go to berkshelf 4. |
change listen attributes to be hash of addresses and ports to listen to
|
Looks good to me! Thanks! We'll have to look into the berkshelf thing... |
|
Sorry for jumping in late here, but somehow I didn't notice the updates to my original PR that triggered this work. Using a hash for the adresses makes using the listen attribute rather complicated, so I'm wondering whether this could get amended to using a flat array of "addr:port" combinations. The use case I'm having is that a recipe wants httpd to listen on addr:port in addition to whatever some other recipes may have configured. Now instead of being able to simply set I would also vote to get rid of setting |
|
@jrosenboom that seems fine to me. My original need here was just to be able to have it listen on more than every address on every port. An array of The only downside I can think of is that arrays don't merge across node attribute levels the same way hashes do. So the apache2 cookbook couldn't have an attributes file with default["apache"]["listen"] = [ "*:80" ]and then later someone in some node.normal["apache"]["listen"] += [ "127.0.0.1:1234" ]because then the normal level wouldn't have the |
This PR amends the logic introduced in sous-chefs#378 to use an array as attribute instead of a hash, as this will simplify using this cookbook in other recipes. E.g., if a recipe wants httpd to listen on `addr:port`, it is much easier to just set node.set['apache2']['listen'] += ['addr:port'] instead of first having to check whether the listen hash does have the key `addr` already and then either add it or update the port list.
This PR amends the logic introduced in sous-chefs#378 to use an array as attribute instead of a hash, as this will simplify using this cookbook in other recipes. E.g., if a recipe wants httpd to listen on `addr:port`, it is much easier to just set node.set['apache2']['listen'] += ['addr:port'] instead of first having to check whether the listen hash does have the key `addr` already and then either add it or update the port list.
|
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. |
This should fix the issue that was going on w/ PR #372
The attributes
node['apache']['listen']will be a hash indexed by address where the values are ports to listen to.For example:
{ "apache": { "listen": { "*": ["80", "443"], "127.0.0.1": [ "12345" ] } } }would create this ports.conf:
Which was impossible with the older listen_ports and listen_addresses attributes.