Reachable via the cache config page, restricted to admins only. This makes it convenient to hotfix phabricator without requiring a restart.
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- Restricted Diffusion Commit
rPae0348aac931: Add dialog to purge opcode/data caches
- Local dev machine doesn't have apc, so I get the not installed message.
- Faked the name and isEnabled parameters, verified dialog shows up as expected.
- Didn't test clear code
Diff Detail
- Repository
- rP Phabricator
- Branch
- purge2
- Lint
Lint Passed - Unit
Tests Skipped - Build Status
Buildable 7964 Build 8957: [Placeholder Plan] Wait for 30 Seconds Build 8956: arc lint + arc unit
Event Timeline
Conceptually fine but I want to smooth it down a bit before upstreaming it. Probably put it here with other cache stuff:
When clicked, pop a confirmation dialog with the caveat that only the current web frontend will be purged. Show "no caches are configured" if nothing is purgeable.
When the user confirms, clear both APC and OPCache (if they're installed), then tell the user which cache(s) were cleared.
That's like 200x more work so feel free to not bother and I'll just commandeer this at some point.
Looks great! Couple of minor simplicity/clarity/fragility things in line, if they make sense.
src/applications/cache/spec/PhabricatorDataCacheSpec.php | ||
---|---|---|
125–134 | This feels a little redundant (both cache specs implement similar code) and a little fragile (changing human-readable text in pht() generally should not affect program behavior, but it will here). How about this instead? Add these to PhabricatorCacheSpec (or pick some better names if these feel too clunky):
Then, when building the caches, use $this->setClearCacheCallback('apc_clear_cache');, etc., to specify how to clear the cache. | |
src/applications/config/controller/PhabricatorConfigPurgeCacheController.php | ||
5 | This should likely extend PhabricatorConfigController instead. It can then inherit the shouldRequireAdmin() implementation automatically, and doesn't need an explicit implementation. | |
16 | Minor nit, but maybe define $cancel_uri = $this->getApplicationURI('cache/'); here to slightly reduce repetition, as this URI is hard-coded in three places in the remainder of the controller. | |
55 | Consider naming the button "Clear Cache" for clarity. |
I think I caught two minor things. This works properly for me locally (with both cache types available).
src/applications/cache/spec/PhabricatorDataCacheSpec.php | ||
---|---|---|
51 | Incorrect / redundant with the call below inside the ini_get() clause? | |
src/applications/config/controller/PhabricatorConfigPurgeCacheController.php | ||
24–25 | If one of these caches is missing but the other is present (e.g., we have OpCache but not APCu) I think this will try to clear a null callback and produce an error like this:
Likely fix is testing if the callbacks are set before invoking them. |