Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

EZP-30554: Purge several keys at once to simplify debug & reduce round trips #85

Merged
merged 4 commits into from
May 21, 2019

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented May 14, 2019

Question Answer
JIRA issue EZP-30554
Type Improvement
Target version 0.9
BC breaks no
Doc needed maybe

As of 2.5LTS we require Varnish 5.1 or higher, meaning we can take advantage of varnish-modules 0.10+ support for several keys in header while purging. In order to:

  • Simplify Varnish debugging when checking purge requests
  • Reduce rountrips on between web server and varnish for purges

Given the package is also allowed to be used on 1.13, which supports Varnish 4.x. Should we ❓

  • A. Expose a setting for this as currently is the case in this PR
  • B. Avoid complexity of a temprary setting, and rather tell 1.13 + Varnish 4.x users to use ezplatform-http-cache 0.8.x.

TODO:

  • Implement feature / fix a bug.
  • Implement tests + specs and passing ($ composer test)
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@andrerom andrerom requested review from vidarl, kmadejski and adamwojs May 14, 2019 18:54
Copy link
Member

@adamwojs adamwojs left a comment

Choose a reason for hiding this comment

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

Given the package is also allowed to be used on 1.13, which supports Varnish 4.x. Should we ❓

My answer is:

B. Avoid complexity of a temprary setting, and rather tell 1.13 + Varnish 4.x users to use ezplatform-http-cache 0.8.x.

src/PurgeClient/VarnishPurgeClient.php Outdated Show resolved Hide resolved
@andrerom
Copy link
Contributor Author

@adamwojs & @vidarl updated

Copy link
Member

@kmadejski kmadejski left a comment

Choose a reason for hiding this comment

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

Just a two tiny nitpicks, besides that 👍

docs/varnish/vcl/varnish5.vcl Outdated Show resolved Hide resolved
src/PurgeClient/VarnishPurgeClient.php Outdated Show resolved Hide resolved
Co-Authored-By: Kamil Madejski <kmadejski@live.com>
@andrerom andrerom merged commit 7c55fed into 0.9 May 21, 2019
@andrerom andrerom deleted the bulk_purge_varnish branch May 21, 2019 08:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants