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

refs #54 nginx-proxy caching #55

Merged
merged 7 commits into from
Sep 11, 2019
Merged

refs #54 nginx-proxy caching #55

merged 7 commits into from
Sep 11, 2019

Conversation

gingerlime
Copy link
Contributor

@gingerlime gingerlime commented Aug 18, 2019

  • adding new image: thumbor-nginx-proxy-cache
  • the new nginx-proxy-cache will replace nginx-proxy with built-in proxy
    caching by nginx instead of mapping to thumbor's data folder
  • refreshed nginx.tmpl from upstream (jwilder/nginx-proxy)
  • updated template to add thumbor + proxy caching with minimal control
    using environment variables (for further customization, users can
    supply a proxy.conf file)
  • updated tests: added proxy caching tests + removed some tests that are
    unnecessary or difficult to do with the built-in proxy caching
    (for example, overriding the cached data is somehow detected by nginx)
  • updated recipes to reflect the new setup (no need for result storage
    caching in thumbor, and no data volume)
  • added deprecation warning to nginx-proxy

* proof-of-concept (WIP) using internal nginx proxy caching
  instead of relying on thumbor's result storage and try_files
Yoav added 3 commits August 25, 2019 10:09
* the new nginx-proxy-cache will replace nginx-proxy with built-in proxy
  caching by nginx instead of mapping to thumbor's data folder
* refreshed nginx.tmpl from upstream (jwilder/nginx-proxy)
* updated template to add thumbor + proxy caching with minimal control
  using environment variables (for further customization, users can
  supply a proxy.conf file)
* updated tests: added proxy caching tests + removed some tests that are
  unnecessary or difficult to do with the built-in proxy caching
  (for example, overriding the cached data is somehow detected by nginx)
* updated recipes to reflect the new setup (no need for result storage
  caching in thumbor, and no data volume)
* TODO: revert changes to nginx-proxy and add deprecation warning
add_header 'Access-Control-Allow-Origin' '{{ or .CorsOrigin "*" }}';
add_header 'Access-Control-Allow-Credentials' 'true';
add_header 'Access-Control-Allow-Methods' 'GET, POST, PUT, DELETE, OPTIONS';
add_header 'Access-Control-Allow-Headers' 'Accept,Authorization,Cache-Control,Content-Type,DNT,If-Modified-Since,Keep-Alive,Origin,User-Agent,X-Mx-ReqToken,X-Requested-With';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

X-Mx-ReqToken what this header is for? Quick search pointed to some java lib/app. Is it sent by browsers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question :) this was lifted from https://github.com/APSL/docker-thumbor/blob/a1bf9bd1bf9520f02fa57ba1a7cd66778edc3f4b/nginx/nginx.conf#L64 originally ... I didn't really look into it at the time and just continued to pass it over... Is it worth refining this?


{{ define "thumbor-caching" }}
{{ $proxyCacheSize := or .ProxyCacheSize "100m" }}
{{ $proxyCacheInactive := or .ProxyCacheInactive "300m" }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think default sizes/times are too small.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I was wondering what's a reasonable default. This is already larger than the nginx defaults, but wasn't sure how far I should go... What would you recommend?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated these values, and also realized I was a bit confused between the memory size used for caching and the (on-disk) max cache size, so added a separate ENV as well.

Yoav added 3 commits September 1, 2019 13:53
* added README for nginx-proxy-cache
* updated recipes to add a persisted cache folder
@gingerlime gingerlime merged commit 6a31a5f into master Sep 11, 2019
@gingerlime gingerlime deleted the nginx-proxy-cache branch September 11, 2019 17:24
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

Successfully merging this pull request may close these issues.

2 participants