wodby / varnish

Varnish docker container image
https://wodby.com/stacks/varnish
MIT License
59 stars 29 forks source link

Drupal cache static files#27 #29

Open drasgardian opened 3 years ago

drasgardian commented 3 years ago

Allows a drupal installation to:

https://github.com/wodby/varnish/issues/27

drasgardian commented 3 years ago

Prior to this change, using the drupal preset only static resources were being cached, and they were only being cached for anonymous users.

Now, public static resources will be cached by varnish for authenticated users too. Private files (e.g. /system/files/*) still won't get cached.

I also added the below to the drupal preset to get caching of page responses for anonymous users working.

sub vcl_backend_response {
    # Set the default TTL
    set beresp.ttl = {{ getenv "VARNISH_DEFAULT_TTL" "120s" }};
}
pprishchepa commented 3 years ago

Hi, @drasgardian, thank you for the PR.

Some questions and thoughts:

  1. Could you squash 2 commits pls
  2. Does this modification respect VARNISH_CACHE_STATIC_FILES setting?
  3. It makes sense to make caching static for authenticated users optional and disabled by default. And looks like it must be another one option because VARNISH_CACHE_STATIC_FILES is about caching for anonymous.
drasgardian commented 3 years ago

Hi @PavelPrischepa

static.vcl.tmpl still checks for VARNISH_CACHE_STATIC_FILES and will return (pass); if not set.

I don't see why caching static files for authenticated users should be a separate configuration.

Requests (in a Drupal site) for static assets in the public filesystem, or in a theme or module are served directly by nginx anyway, without any PHP processing or authentication checks. So if caching of these static files is enabled for anonymous users, there shouldn't be any reason not to also cache for authenticated.

drasgardian commented 3 years ago

@PavelPrischepa 2 commits squashed into one as requested.