Problem/Motivation

Quoting #2889603-17: Split the internal page cache from the rest of the render cache:

Furthermore… #2775381: response.gzip is redundant and should be removed just landed. Which means we don't have any of this "page compression" stuff anymore. So it's completely obsolete. #2775381 failed to remove it because… there was zero test coverage for this.

Proposed resolution

  1. Remove the obsolete logic in PerformanceForm::submitForm()
  2. Also remove the now obsolete injected "render cache" service.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 2892179-2.patch3.09 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2892179-2.patch3.09 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Makes sense, I guess it is unlikely that someone subclassed that form and has a broken constructor now.

What's possibly a bit more likely is that someone altered something into that form and relied on the invalidation. I actually have a custom module module that might in theory by affected by that (custom caching headers for fastly, but it's not worth invaliding caches to cache things longer, so...) but it was already unreliable for most cases due to the separate dynamic page cache bin for example.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2892179-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mpdonadio’s picture

mpdonadio
he/him
Primary language English
Location Philadelphia/PA/USA (UTC-5)
commented
Status: Needs work » Reviewed & tested by the community

Believe we just had a random from #2857843: Random fail in Drupal\KernelTests\Core\Entity\ContentEntityChangedTest::testChanged. Back to RTBC.

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

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

  • catch committed a8a8984 on 8.5.x 
    Issue #2892179 by Wim Leers, Berdir: Follow-up for #2775381: clearing...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.5.x and 8.4.x, thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Yay! Only pushed to 8.5.x though :)

  • catch committed 84d9542 on 8.4.x 
    Issue #2892179 by Wim Leers, Berdir: Follow-up for #2775381: clearing...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Hah, there it is!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.