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

Drupal cache static files#27 #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

drasgardian
Copy link

Allows a drupal installation to:

  • Cache public static files, even for authenticated users.
  • Cache page responses for anonymous users, but not authenticated.

#27

@drasgardian
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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.

cache public static files from drupal
@drasgardian
Copy link
Author

@PavelPrischepa
2 commits squashed into one as requested.

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