Problem/Motivation

When I set a certain block to be cached for up to e.g. 15 minutes, then I expect that the containing page also emits a corresponding header. And I also expect Drupal's page cache to honor this.

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

None.

API changes

None.

Comments

catch’s picture

In principle I think supporting this use case is OK.

In practice I'm not sure about it.

The Drupal 7 model was that block caching was really about authenticated users, and page caching was for anonymous, and these were separate things. It opens up the potential for contrib or custom modules (especially ones that don't ignore/don't understand cache tags) to break page caching in fun and unexpected ways.

Wim Leers’s picture

Issue tags: +page rendering

Good points.

This is indeed more about the principle than saying "we MUST do this for 8.0.0". It's a nice-to-have. It'd be consistent with the reasoning we want developers to apply when thinking about page rendering.

Wim Leers’s picture

Title: Bubbling of max-age to the page's headers and the page cache » Bubbling of blocks' max-age to the page's headers and the page cache

Clarifying the main use case.

Fabianx’s picture

So a clear yes from me:

This is why #cache['ttl'] or max-age is independent from #cache expires.

I think we can easily combine this with the place holder approach, but bubbling should probably be configurable.

But having that information besides the cache context is important to know if we want to render a place holder.

Wim Leers’s picture

#4: Yep. But making the bubbling configurable sounds problematic/tricky/confusing to me.

FYI: CacheableInterface already works with a max-age (i.e. relative time), not with an expiration time (i.e. absolute time), so we've already paved the way partially :)

catch’s picture

So I definitely think we can make this a developer decision - always bubble up max_age. Then it's OK to file a bug report if people are doing it wrong.

Where there's a UI for setting the cache time of a block, then that should probably also include whether it should affect parent/page caching ttl too - since that's where people will get into trouble (thinking of the Views UI specifically here).

beejeebus’s picture

when there are multiple blocks on a page with different max-age values, i'm assuming we want the smallest one to win? perhaps we should put that in the summary.

Fabianx’s picture

#7 The smallest one that we want to bubble up - yes.

Wim Leers’s picture

Fabianx’s picture

Not to the page cache though what this was originally about ;).

Fabianx’s picture

Status: Closed (duplicate) » Active
Wim Leers’s picture

Well, it actually is bubbled to the response, but is then overwritten by FinishResponseSubscriber.

But sure, let's use this issue to fix that overwriting problem :)

Fabianx’s picture

catch’s picture

Are we sure we want to override the internal page cache? I'm not completely opposed but that feels tricky to explain how it works with the max_age setting. For example render cached content is assumed to be permanent if there's no max age set, so the setting would represent a cap on that - but then if someone sets max_age to a week in #cache and the config is set to 10 minutes, which wins?

Wim Leers’s picture

#14: I think that's super easy to explain actually: that setting represents the default max-age. Therefore it applies to pages that are marked as permanently cacheable. In other words: min(bubbled max-age, configured default max-age).

Wim Leers’s picture

mr.baileys’s picture

Assigned: Unassigned » mr.baileys
Issue tags: +drupaldevdays
Fabianx’s picture

Currently the max-age is already set on the Response object via setMaxAge(), the page cache can use that setting.

mr.baileys’s picture

If I understand correctly, what is requested in this issue is actually what we are removing in #2467041: max-age on HTML responses wrongly set to `max-age=0, private` instead of `max-age=N, public` (breaks reverse proxies and client-side caching). Until that issue lands, max-age is actually bubbled up the stack and the smallest element max-age value makes it into the cache-control header. However, since some elements are setting max-age=0, this ends up marking each page request as "max-age=0, private".

So, is this issue about:
a) re-enabling what we are removing in #2467041: max-age on HTML responses wrongly set to `max-age=0, private` instead of `max-age=N, public` (breaks reverse proxies and client-side caching) after all elements correctly set max-age <> 0
b) changing the way we handle previously set cache headers in FinishResponseSubscriber::onRespond()

In any case, it seems that this issue is postponed until we have fixed max-age being set to 0 on some elements that are currently causing the entire page to have max-age=0?

mr.baileys’s picture

Title: Bubbling of blocks' max-age to the page's headers and the page cache » Bubbling of elements' max-age to the page's headers and the page cache
Wim Leers’s picture

Title: Bubbling of elements' max-age to the page's headers and the page cache » [PP-1] Bubbling of elements' max-age to the page's headers and the page cache
Assigned: mr.baileys » Unassigned
Status: Active » Postponed
Issue tags: +D8 Accelerate Dev Days
Related issues: +#2469431: BigPipe for auth users: first send+render the cheap parts of the page, then the expensive parts

Yes, you're right. Sorry about that. We can do this as soon as #2469431: BigPipe for auth users: first send+render the cheap parts of the page, then the expensive parts lands, or when all blocks that actually are cacheable, but aren't yet due to unfixed issues. Whichever is done first.

yched’s picture

Coming from #2429617-270: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!).
SmartCache puts debug info in its own X-Drupal-SmartCache header, with values HIT/MISS, and UNCACHEABLE when max-age = 0.

This issue here is where we should, similarly, output UNCACHEABLE in X-Drupal-Cache when applicable ?

Wim Leers’s picture

Title: [PP-1] Bubbling of elements' max-age to the page's headers and the page cache » Bubbling of elements' max-age to the page's headers and the page cache
Version: 8.0.x-dev » 8.1.x-dev
Component: cache system » request processing system
Status: Postponed » Active

Now that we have #2476407: Use CacheableResponseInterface to determine which responses should be cached (since yesterday), this has become more possible than ever. And I don't think this is actually blocked on #2469431: BigPipe for auth users: first send+render the cheap parts of the page, then the expensive parts — I suspect that was either the wrong issue ID, or that originally had a different intent.

But, I think this will have to wait until 8.1.

DuaelFr’s picture

I opened #2592555: Time based cache does not work as expected for anonymous users today that seems to be related to this issue (thanks @catch to have pointed this one to me).

What I understand here is that if I want to implement the very common use case of a piece of generated content with a time based cache (i.e. a RSS aggregation or a randomized view), anonymous users are going to always see the same thing unless I manually clear the cache.
Am I right?

Wim Leers’s picture

You're right, because the PageCache middleware does this:

    // Use the actual timestamp from an Expires header, if available.
    $date = $response->getExpires()->getTimestamp();
    $expire = ($date > time()) ? $date : Cache::PERMANENT;

    $tags = explode(' ', $response->headers->get('X-Drupal-Cache-Tags'));
    $this->set($request, $response, $expire, $tags);

i.e. it only respects Expires in HEAD, not Cache-Control: max-age.

Which is silly.

Wim Leers’s picture

Berdir’s picture

It is kind of silly, but you sold that to me as a feature a while ago :)

Having a separate external and internal max age can definitely be a useful feature, especially if you can't invalidate the external cache. You can set it to a short time, so that it frequently re-validates using the internal page cache.

catch’s picture

I think we can do that with an X-Internal-Page-Cache-Max-Age header specially for that purpose. Akamai has similar with Edge-control, and Wim pointed out in irc there is s-max-age.

Fabianx’s picture

#30: Yes, Symfony and the FosHttpCache bundle uses s-max-age for that purpose, e.g. $response->setSharedMaxAge(600);

Wim Leers’s picture

#29: I'm sure that was only jokingly then :P

The problem with s-maxage is that it will be respected by ALL proxies. i.e. also those we cannot invalidate using cache tags.

For that purpose, CDNs like Akamai and Fastly have the Surrogate-Control header, which is like Cache-Control but specifically targeted at the infrastructure you control. That infrastructure then strips that header. But since that's already used by such external services, I think something like Page-Cache-Control would make more sense: it'd target Page Cache specifically.

Berdir’s picture

I'm a bit confused why we talk about headers. If you are able to set a header, then you can already set the Expires header and page cache will respect that? The problem IMHO is when you can't but want it to respect whatever max-age was set by a block, statistics.module, a views or whatever.

Isn't what we want here exactly what you implemented in #2527126-143: Only send X-Drupal-Cache-Tags and -Contexts headers when developer explicitly enables them? To be clear, I'm not against doing that at all, just against doing it in that issue. That would finally allow to properly use statistics.module also for anonymous users, something that in 7.x only worked by setting the page cache expiration manually to 1h or whatever frequency you wanted to have it updated. And right now doesn't work at all.

But we need to make sure that we no longer send out max-age 0 in common cases, as that would avoid those pages from being cached. Maybe we should only respect a value != 0. It's a bit weird but would be a minimal behavior change compared to HEAD...

catch’s picture

I'm a bit confused why we talk about headers. If you are able to set a header, then you can already set the Expires header and page cache will respect that?

You can but then Expires gets passed to everything after page cache too. Same with max-age once we make page caching respect max-age.

Wim Leers’s picture

#33 I was replying to #29+#30+#31, which were talking about "different max-age for proxies".

Isn't what we want here exactly what you implemented in […]

Yes, it is!

Fabianx’s picture

Version: 8.1.x-dev » 8.0.x-dev
Category: Task » Bug report

#28: Per marking #2592555: Time based cache does not work as expected for anonymous users as a duplicate of this, that makes this essentially a 8.0.x bug.

If we don't want this, we need to split this out again.

aNickPlx’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Component: request processing system » page_cache.module
Berdir’s picture

IMHO, before considering this, I think we need to introduce a concept to say that e.g. a block is not worth caching. Right now, you can achieve that by returning max-age 0, but it's not the same. As the result would be that page cache would be disabled.

For example something like \Drupal\system\Plugin\Block\SystemPoweredByBlock, it's totally pointless that cache a hardcoded #markup string , just calling it will always be faster since we have to instantiate the block anyway. But you still want it to be cached in the page cache.

Also, at this point this would IMHO be an API change?

Fabianx’s picture

#40: I think you can now do this within system module via https://api.drupal.org/api/drupal/core%21modules%21block%21block.api.php....

Just unsetting the #cache 'keys' is enough.

I still would love to have a $block->alterBuild() call to do the same as the hook, but Wim was against it so maybe in 8.9.x ;).

So not worth caching is taken care of and yes probably the new behavior would be opt-in via a $conf flag or such, e.g. maybe even enabled for new installations ...

davidwbarratt’s picture

Component: page_cache.module » cache system
deviantintegral’s picture

Status: Active » Needs review
FileSize
1.01 KB

I just ran into this as a part of #2772847: Add support for private uploads / presigned URLs. I'm a bit lost in all of the discussion, but is there any need for this to be more complex then this patch? At the least, it's working for me in the basic case.

Status: Needs review » Needs work

The last submitted patch, 43: 2352009.43-bubble-max-age.patch, failed testing.

Berdir’s picture

The test fails show why this is a problem. There are max-age 0 elements on many pages, doing this results in them no longer being cacheable. That's a behavior change that I don't think we can just introdue like that.

I've commented about this multiple times before :)

As a start, we need to at least figure out what max-age 0 elements we have and figure out what to do about them. Then respecting this could be an option, I don't know..

andypost’s picture

That's all depends on "strategy" used - all places are replaced with placeholder to make whole page cachable if I remember right

Wim Leers’s picture

#46: not everything can be placeholdered. Only things that are rendered using a #lazy_builder.

Berdir’s picture

And more importantly, we are talking about the internal/anon page cache, that doesn't support that anyway, only the dynamic page cache does :)

Fabianx’s picture

#48 I think the only way to fix this is to be honest about it and remove the max-age=0 e.g. for forms when used by the anonymous user, etc.

And all other places, too, where we currently allow caching in page cache and external proxies.

Because if we allow that caching, then we should not be declaring max-age=0 in the first place (for anon user).

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

Status: Needs work » Needs review
FileSize
1.32 KB

So the patch above didn't actually do anything useful yet, because max-age 0 was skipped and ignored (which should result in it not being cacheable) while a -1 actually resulted in expire being lower than request time and was then not cached.

The 4xx caching expire also changed this quite a bit, so didn't apply anymore.

New patch that should work correctly with 0 and -1. Also checking expires first, that's more backwards compatible I think.

Lets see how many test fails we have now. The aggregator test is now actually passing.

Status: Needs review » Needs work

The last submitted patch, 51: bubbling_of_elements_-2352009-51.patch, failed testing.

Berdir’s picture

Version: 8.2.x-dev » 8.3.x-dev
Status: Needs work » Needs review
FileSize
2.56 KB
1.24 KB

Interesting, was wondering if that was going to happen. That means we're actually doing a much better job at setting max-age than I thought. Good. The question is how many pages we are not testing.

The only fail we have is in GlossaryTest, because that view is marked as uncacheable and that now bubbles up. And the test actually doesn't care about testing that the page is cached, it just uses the standard method from the trait to assert this. And the cache tags/contexts are actually there on the response, the problem is just that there is no cache entry now anymore for this view.

Also weird is that \Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait::assertPageCacheContextsAndTags() does a negative MISS check on the first request, but it does *not* check for a HIT on the second. But it would implicitly fail as shown in this example.

The reason why the glossary view is not cacheable is interesting, though. And that's \Drupal\views\Plugin\views\style\Table::getCacheMaxAge(). That returns 0 and I can't really say why? Best I can tell is that this is actually a bug that we introduced in #2464427: Replace CacheablePluginInterface with CacheableDependencyInterface. We went from isCacheable() TRUE to max-age 0.

That means we have two ways of fixing this test. I first converted the test to not use that trait, but by fixing the table style plugin, the test actually passes unchanged.

Wim Leers’s picture

does a negative MISS check on the first request, but it does *not* check for a HIT on the second

Ugh, that looks like a big, sad mistake :( Did some archeology: #2381217: Views should set cache tags on its render arrays, and bubble the output's cache tags to the cache items written to the Views output cache introduced that code, and there, we did check the first request was a MISS and the second a HIT. Then came along #2477157: rest_export Views display plugin does not set necessary cache metadata and it refactored things, and it seems that's how it was lost. I failed to notice that.

The reason why the glossary view is not cacheable is interesting, though. And that's \Drupal\views\Plugin\views\style\Table::getCacheMaxAge(). That returns 0 and I can't really say why? Best I can tell is that this is actually a bug that we introduced

+1 for being an accidental bug. I re-read that patch, and indeed, the Table row style is the exception: everything went from TRUE to Cache::PERMANENT and from FALSE to 0, except this one.

That means we have two ways of fixing this test. I first converted the test to not use that trait, but by fixing the table style plugin, the test actually passes unchanged.

Fixing the table style plugin totally makes sense to me. If people feel uncomfortable doing that here, it could easily be spun off into its own issue.


+++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php
@@ -284,8 +284,19 @@ protected function storeResponse(Request $request, Response $response) {
+        $expire = $max_age > Cache::PERMANENT ? $request_time + $max_age : Cache::PERMANENT;

I'm very wary of using > Cache::PERMANENT or < Cache::PERMANENT. I'm only comfortable with === Cache::PERMANENT. I don't see why we can't make this $expire = $max_age !== Cache::PERMANENT ? $request_time + $max_age : Cache::PERMANENT;


I think this patch looks splendid. I'd love to RTBC, but … I think we must be overlooking something? I remember this issue feeling unsolveable without breaking BC. Do we need additional tests to ensure we're not breaking BC?

pfrenssen’s picture

This looks like it will cause all pages in my current project to become uncacheable in a very fun way.

I have a couple of facet blocks in my search page, and these have max-age = 0. I have noticed that this currently causes all my pages to be marked with max-age = 0, not just the search page. At the moment this is not a huge problem since, hey, the page cache still caches pages with max-age = 0 at the moment! :D

I could not find an issue about it so I created #2828136: A single uncacheable block can make the entire website uncacheable. We might have a look at this since this will mean that merging this issue will affect every site that uses uncacheable blocks, for example because they use the Facets module.

Berdir’s picture

Commented there, but yes, I think that is the main thing that we have to worry about. Core is either doing pretty fine or we simply don't have the explicit assertions to detect unexpected page cache misses. But who knows that will happen to actual sites.

What we could do is declare it an (experimental?) opt-in behavior that you have to enable with a setting or so. And declare that we'll make it the default in 9.x. Kind of the opposite of a deprecation. And speaking of that, if this would just work, it would mean that we could replace most if not all use cases of the kill switch service with setting max-age on the response/some render array and eventually deprecate that?

pfrenssen’s picture

I think that is the main thing that we have to worry about. Core is either doing pretty fine or we simply don't have the explicit assertions to detect unexpected page cache misses. But who knows that will happen to actual sites.

What we could do is declare it an (experimental?) opt-in behavior that you have to enable with a setting or so. And declare that we'll make it the default in 9.x. Kind of the opposite of a deprecation. And speaking of that, if this would just work, it would mean that we could replace most if not all use cases of the kill switch service with setting max-age on the response/some render array and eventually deprecate that?

I agree that we cannot predict all cases that will happen in live sites, but even though I did find #2828136: A single uncacheable block can make the entire website uncacheable which would turn my entire site uncacheable, I actually think it's fine to fix this for real. I don't think we should default to the buggy behaviour and opt-in to the fixed behaviour. I fear that this will just complicate things in the long run. The current behaviour is just wrong.

I do think we need enough lead time to discover other potential caching problems that are now hidden because of this issue. 8.3.0 is scheduled for April next year, if we can get this in before the end of December this will probably give enough time. After that, we should maybe consider a kill switch or postpone it to 8.4.x.

bkosborne’s picture

Doesn't the current patch fail to account for FinishResponseSubscriber.php, which currently has this code:

    $max_age = $this->config->get('cache.page.max_age');
    $response->headers->set('Cache-Control', 'public, max-age=' . $max_age);

The max age is always taken from the core config setting.

EDIT: The issue I described is documented in #2732129: FinishResponseSubscriber::setResponseCacheable() does not respect Cacheable Metadata for Cache-Control header, but as it stands it's not clear if this issue is intended to fix that as well as the Page Cache module, or just the Page Cache module.

catch’s picture

We should split this into two issues:

1. For deciding on and implementing support for a new #cache property which can be used to set s-max-age or Surrogate-Control or similar.

2. For bubbling max-age itself (except I don't think we should do that because it could very easily break existing sites).

DuaelFr’s picture

About Max-age bubbling we could create a checkbox in the performance settings to enable this option then have an upgrade path that would disable it for existing sites to avoid breaking BC. That setting could be marked as deprecated and removed in D9 (or not).

pfrenssen’s picture

+1 for @DuaelFr's idea of providing this as a configuration option which is disabled for existing sites and enabled for new installations. I don't think it is necessary to make this option visible in the UI, but if we do this is then the performance settings are indeed ideal for it.

vijaycs85’s picture

we are trying to solve two different problems here (at least in code level):
1. Adding expire to page cache - gets saved in a storage to serve when Drupal receives a request
Current: This has been done by the value of response's 'expires' header. If there is no 'expires' header, cached permanently.
Solution: patch #53 allows to fallback to bubbled-up max age, which is set by setCacheMaxAge(),

2. Adding max-age header - Allows other systems (browser, cdn, reverse proxy) in front of drupal/server to deal with the response caching
Current: If a page is cacheable, use the config value system.performance:cache.page.max_age to set max-age in cache-control header
Solution: We have two parts mentioned in #59 here. We can use #2732129: FinishResponseSubscriber::setResponseCacheable() does not respect Cacheable Metadata for Cache-Control header for 59.1 and this issue for 59.2 AND page_cache expire.
As per #60 and #61, we can provide an option, so that user can select to use setCacheMaxAge() instead of fixed system.performance:cache.page.max_age

Below patch adds option in performance page and use it as max-age in response header. Now it's possible to fallback on getCacheableMetadata()->getCacheMaxAge() for both page_cache and max-age on Cache-Control header.

Status: Needs review » Needs work

The last submitted patch, 62: 2352009-62.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
7.62 KB
560 bytes

Quick schema update...

Status: Needs review » Needs work

The last submitted patch, 64: 2352009-64.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
8.23 KB
629 bytes

Fixing last test fail.

Wim Leers’s picture

This is looking good, but it's not ready yet! An update path is missing for example.

  1. +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php
    @@ -227,7 +228,16 @@ protected function setResponseCacheable(Response $response, Request $request) {
    +    if ($this->config->get('cache.page.max_age_provider') == 'dynamic') {
    ...
    +      if ($max_age == Cache::PERMANENT) {
    

    ===

  2. +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php
    @@ -227,7 +228,16 @@ protected function setResponseCacheable(Response $response, Request $request) {
    +        // Cache for 30 days.
    +        $max_age = 30 * 24 * 60 * 60;
    

    Why 30 days?

  3. +++ b/core/modules/node/config/optional/views.view.glossary.yml
    @@ -355,7 +355,7 @@ display:
    -      max-age: 0
    +      max-age: -1
    
    +++ b/core/modules/views/src/Plugin/views/style/Table.php
    @@ -425,7 +426,7 @@ public function wizardSubmit(&$form, FormStateInterface $form_state, WizardInter
    -    return 0;
    +    return Cache::PERMANENT;
    

    This seems to be an unrelated fix.

  4. +++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php
    @@ -264,8 +264,19 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch
    +      else {
    +        $max_age = $response->getCacheableMetadata()->getCacheMaxAge();
    +        // Fall back to checking the max age, which can either be unlimited, in
    +        // which case it should be used as-is, a positive value for a certain
    +        // expiration date or 0 if the page is not cacheable. 0 means that
    +        // expire is the request time, which means it will not be cached below.
    +        $expire = $max_age > Cache::PERMANENT ? $request_time + $max_age : Cache::PERMANENT;
    +      }
    

    Not every response has cacheability metadata. So you need to wrap this in else if ($response instanceof CacheableResponseInterface) {…} and then keep the original else.

  5. +++ b/core/modules/system/src/Form/PerformanceForm.php
    @@ -118,6 +118,16 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    +        'fixed' => t('Fixed'),
    +        'dynamic' => t('Dynamic'),
    

    I'm not sure these are the appropriate labels.

    "fixed" is really "override": override whatever the bubbled max age is, and use this fixed value anyway.

    "dynamic" is really "computed", "calculated" or "bubbled": it matches the rendered content's max-age.

  6. +++ b/core/modules/system/src/Form/PerformanceForm.php
    @@ -118,6 +118,16 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    +      '#default_value' => $config->get('cache.page.max_age_provider') ?: 'fixed',
    

    The ?: 'fixed' should not be necessary if there is a working upgrade path.

  7. +++ b/core/modules/system/src/Form/PerformanceForm.php
    @@ -129,6 +139,11 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    +      '#states' => array(
    +        'visible' => array(
    +          'select[name="page_cache_maximum_age_provider"]' => array('value' => 'fixed'),
    +        ),
    +      ),
    

    Nice!

vijaycs85’s picture

Thanks @Wim Leers, for the review.

#67.1 - FIXED
#67.2 - NOT FIXED: Not sure how to set permanent. probably we shouldn't set max-age derivative at all?
#67.3 - NOT FIXED: Yeah, its for page cache. @Berdir mentioned, we might split the views related changes to different issue. probably leave it for now.
#67.4 - FIXED.
#67.5 - NOT FIXED: hmm, I know I wasn't totally happy about the labels either. Should we do ['override' => 'Fixed period', 'bubbled' => 'Dynamic']
#67.6 - NOT FIXED: we have hook_update_N. we might need to change the number, but it would work right?
#67.7 - :)

Status: Needs review » Needs work

The last submitted patch, 68: 2352009-8.3.x-68.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
9.03 KB
627 bytes

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Wim Leers’s picture

Status: Needs review » Needs work

This still needs an update path.

#67.6: well if it works, hen you should be able to remove that ?: 'fixed' portion!

vijaycs85’s picture

FileSize
9.03 KB

Thanks @Wim Leers. Updated for #67.6 and re-rolled.

Wim Leers’s picture

Can you provide an interdiff?

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
315.58 KB

@Wim Leers It was a reroll as the patch in #70 was against 8.2.x. However here is the diff-ui (after all we have UX initiative in core :D)

rgpublic’s picture

What I don't understand (and I literally mean "don't understand" - it's not a rhetoric question or rant or anything): How could Drupal 8 have ever been released without that feature? Why is there some talk above in this issue about it being "nice-to-have"? Why do I diligently set my max-age-keys within my preprocess functions of paragraphs, views all the time only to find out they have no effect whatsoever, because the page cache is overriding all of this anyway? It's totally unexpected. Or was the full-page-cache never intended to be used at the same time with the dynamic page cache caching individual parts? Was it my mistake to enable both or expect both of them to work at the same time? Would be great if someone could enlighten me :-)