Adding support for include in map and support some proxy_cache options #1145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- use include directive in map - proxy_cache_bypass - proxy_cache_lock
adding comments on file headers
ceonizm
changed the title
alexjfisher
reviewed
Nov 15, 2017
templates/conf.d/map.erb
Outdated
| @@ -6,6 +6,11 @@ map <%= @string %> $<%= @name %> { | |||
| <% if @default -%> | |||
| default <%= @default %>; | |||
| <% end -%> | |||
| <% if @includes.is_a?(Array) -%> | |||
There was a problem hiding this comment.
Can it ever not be an array?
alexjfisher
reviewed
Nov 15, 2017
templates/server/locations/proxy.erb
Outdated
| @@ -46,4 +46,12 @@ | |||
| <% if @proxy_cache_key -%> | |||
| proxy_cache_key <%= @proxy_cache_key %>; | |||
| <% end -%> | |||
| <% if @proxy_cache_bypass && Array(@proxy_cache_bypass).size > 0 -%> | |||
There was a problem hiding this comment.
Initially when I've added theses features I've started from the 0.5.0 version which was published on forge and followed the same coding style, that's why there is this check I believe:
I totally agree with you it seems useless now.
|
@ceonizm thanks! Would it be possible to split this PR up by feature you’re adding? Eg support for access log in a location seems unrelated to the proxy_cache changes. |
This was referenced Nov 16, 2017
|
I close this PR and open some new one from different branches in order to get all features splitted. |
|
@ceonizm Thanks. This will make it easier to review and get your additions merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
I needed to use the include directive in a map block to handle some redirections that were dynamically generated by an other app. I noticed this feature was missing from your module so I've added it.
I've also added the support for proxy_cache_bypass and proxy_cache_lock
Hopes it helps.
Best Regards
François Boukhalfa