CLI-1810: run sanitize before cache clear.#2003
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2003 +/- ##
=========================================
Coverage 92.42% 92.43%
- Complexity 1957 1960 +3
=========================================
Files 123 123
Lines 7093 7102 +9
=========================================
+ Hits 6556 6565 +9
Misses 537 537 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/2003/acli.phar |
There was a problem hiding this comment.
Pull request overview
This PR updates the post-pull:database script behavior to (1) run drush sql:sanitize before cache rebuild and (2) optionally skip cache rebuild via a new CLI flag, which supports workflows where update hooks must run before caches are rebuilt.
Changes:
- Added
--no-cache-cleartopull:databaseto skipdrush cache:rebuildwhile still runningdrush sql:sanitize. - Reordered script execution so
drush sql:sanitizeruns before cache rebuild. - Added a PHPUnit test covering the
--no-cache-clearbehavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Command/Pull/PullDatabaseCommand.php |
Adds --no-cache-clear and updates script execution logic to sanitize first and conditionally rebuild caches. |
src/Command/CommandBase.php |
Reorders shared “execute all scripts” flow to sanitize before cache rebuild. |
tests/phpunit/src/Commands/Pull/PullDatabaseCommandTest.php |
Updates existing test expectations and adds coverage for skipping cache rebuild. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $this->runDrushSqlSanitize($outputCallback, $this->checklist); | ||
| if (!$noCacheClear) { | ||
| $this->runDrushCacheClear($outputCallback, $this->checklist); | ||
| } |
There was a problem hiding this comment.
Why limit noCacheClear with noScript? it should act regardless of the script or noScript.
There was a problem hiding this comment.
this is if a user specifically run drush cr with --no-cache-clear. we are providing a new option to them
There was a problem hiding this comment.
But the same option will work if you keep it out of the if/else case. That way it can be used in drush cr without as well.
There was a problem hiding this comment.
Ohh that makes sense if there is no option provided, we should keep that in a separate if. Let me refactor this
deepakmishra2
requested review from
anujkaushal,
kalindi-adhiya and
vishalkhode1
This pull request adds a new
--no-cache-clearoption to thepull:databasecommand, allowing users to skip thedrush cache-rebuildstep after pulling the database. This is useful in scenarios where cache should not be cleared immediately, such as when pending update hooks need to run first. The logic for script execution is updated accordingly, and a new test is added to verify this behavior.New Feature:
--no-cache-clearoption to thepull:databasecommand to allow skippingdrush cache-rebuildafter pulling the database. This is helpful when update hooks must run before cache is cleared.Script Execution Logic:
PullDatabaseCommandto respect the new--no-cache-clearflag, ensuringdrush sql-sanitizestill runs whiledrush cache-rebuildis skipped if the flag is set. [1] [2]Testing:
testPullDatabaseNoCacheClearto verify that when--no-cache-clearis used,drush sql-sanitizeruns butdrush cache-rebuilddoes not.