Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

BLT-5206: Remove cache.php, no longer needed for drush #4664

Closed
bkosborne opened this issue Mar 8, 2023 · 5 comments · Fixed by #4665
Closed

BLT-5206: Remove cache.php, no longer needed for drush #4664

bkosborne opened this issue Mar 8, 2023 · 5 comments · Fixed by #4665
Labels
Enhancement A feature or feature request

Comments

@bkosborne
Copy link
Contributor

bkosborne commented Mar 8, 2023

Is your feature request related to a problem? Please describe.
BLT includes this cache.php file which can be used to generate a temporary directory path for drush to use when running drush commands. This was done to work around issues with code deployments in Acquia where another drush command may run at the same time (and thus use the same cache dir, causing issues). See #2867

However, Drush 11 removed its own internal caching entirely:

This file shouldn't be required anymore. BLT 13.x requires at least Drush 11 so should be safe to remove. From what I can tell, this file isn't used within BLT at all, but is used within the split-off ACSF plugin. I created an issue there to remove its usage there as well: acquia/blt-acsf#12

To be clear, the file isn't causing any problems that I can think of, but it's just old code that's not needed.

Describe the solution you'd like
Remove this file

Additional context
I've been debugging why we have cache clear issues in ACSF deploys and I went down a rabbit hole and this drush cache stuff was found along the way.

@bkosborne bkosborne added the Enhancement A feature or feature request label Mar 8, 2023
@github-actions github-actions bot changed the title Remove cache.php, no longer needed for drush BLT-5206: Remove cache.php, no longer needed for drush Mar 8, 2023
@danepowell
Copy link
Contributor

Thanks. I haven't thought about this file in a while but if it's no longer needed I'm happy to remove it: #4665

I couldn't find it referenced anywhere else, do you know if it's part of an include or require anywhere?

@bkosborne
Copy link
Contributor Author

Could be used by BLT plugins. The one for sure that's using it is the ACSF plugin

@bkosborne
Copy link
Contributor Author

@danepowell Might want to first get this removed in the ACSF plugin. As is, if people upgrade BLT, their deployments will break as the ACSF plugin tries to use this file.

@danepowell
Copy link
Contributor

@bkosborne could you please open a PR for blt-acsf? Otherwise I'll need to revert this. I don't use, maintain, or test that plugin. I can only review community PRs.

@anairamzap-mobomo
Copy link

Hi, just a note to say that indeed removing the cache.php file breaks acsf deployments using a db-update script as provided by the Acquia docs: https://docs.acquia.com/site-factory/extend/hooks/post-site-update/

I can see the code itself in the repo https://github.com/acquia/blt-acsf/blob/master/factory-hooks/db-update/db-update.sh was updated, but it would be great to also update the documentation.

Just posting this comment here in case it helps someone else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants