-
Notifications
You must be signed in to change notification settings - Fork 14
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
Include binaries during ddev start #172 #207
Conversation
Talked about this today:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the issues we were having with v3.0.0 on Lullabot.com
…obal binary for DDEV
I believe we've hit our multidev limit or similar as we're getting errors running wipe that are unrelated to this PR: https://github.com/Lullabot/drainpipe/actions/runs/8297688089/job/22709329644?pr=207 |
I've noticed that on other PRs as well. I've been going in and manually deleting the oldest MultiDevs and re-running jobs for Renovate PRs. I just merged a Renovate pull request that all checks passed and then re-ran the action for this one. |
@deviantintegral I've asked Pantheon support chat to bump us to 15 MultiDev and they said we'd have to have a Gold level account. I told them we were a Pantheon partner and they've done this for Lullabot.com which is on a Small tier, they still said they wouldn't do it. Maybe there's someone else we can ask? |
Looks like this is now failing because of a real error during the deploy to Pantheon. https://github.com/Lullabot/drainpipe/actions/runs/8297688089/job/22710408917?pr=207#step:15:544
|
Not sure how the test failure is related. Sadly the drush command doesn't give more information (that message has been fixed in HEAD) drush-ops/drush#5881 |
I've re-tested and now getting a different error (terminus missing a sitename argument). |
@justafish @deviantintegral @davereid Any reason to not update this branch to the latest in main? Or the latest in the 3.7.0 release?? https://github.com/Lullabot/drainpipe/releases I thought of this, cause we went to use #470 on a project that we are pulling in like |
@YesCT hopefully this branch won't be needed anymore with the last round of bug fixes in 3.7.1 and adding |
With 3.7.1, on tugboat, I ran into:
Though we aren't using the new tugboat drainpipe integration. |
@justafish For the filesystem.php error, @leonel-lullabot was able to bypass the error. (By running the init steps again on the preview I think.) So, no known concerns from me about closing this. |
@YesCT It could be an issue on CI, where you don't have the option for "rerun again". |
LSM hasn't run into this issue lately. We may be able to close this. |
Following up from #207 (comment), we think this is solved by adding I don't know if that makes sense for us to ship automatically, or to just add as documentation. Regardless, I'm going to close this PR - but I'll leave the branch in place in case anyone is pointing to it. Thanks all for your efforts on this one! |
Workaround: add composer install as a ddev post-start hook
#164 came up on two separate projects again within a day. As well, we noticed that the error still happens even when not running parallel jobs, which we had previously thought was a workaround.
I know we had left #172 at "needs discussion", but I had a good idea from other work how to approach this, so I figured a PR gave us something to talk about instead of something theoretical.
This PR adds a composer config to disable the current behaviour of downloading task and local-php-security-checker. That way, there's no immediate change for current sites, and they can migrate any existing CI or build scripts one tool at a time.
This PR should also fix:
As well, it fixes users accidentally running composer on the host, getting the host OS binary, and then being confused with the binary not working inside of ddev.
Finally, it upgrades local-php-security-checker because version 1 isn't available for ARM macs.