Make wipe optional in Pantheon review action
The pantheon review action (vendor/lullabot/drainpipe/scaffold/github/actions/pantheon/review/action.yml) currently always wipes an env if it exists and reloads the database from live. In some workflows, this is counterproductive as database-side configurations need to be scaffolded to test the PR effectively.
This step should be optional, but on by default. A variable passed to the action can be used to control if the wipe behavior is skipped or not. This could in turn, be stored as a Github repository secret.
@tess-ten7 👋 do you have any examples of scenarios where this is counter productive? In general we try to keep things as "repeatable" as possible i.e. a PR rebuild would always start things from the same state and configure everything as necessary, rather relying on the database to be in a specific special state as this creates an environment that won't necessarily match what happens on deployment
@justafish There are two use cases which come to mind:
- The upstream database/files are rather large, drastically increasing our Github action costs while those sync.
- The feature being worked on in the PR requires special content to be built to support it, which cannot be implemented on live at this time.
Making the wipe optional allows us to handle both of these issues by deferring the need to wipe and resync to the developer.
@justafish I agree with @tess-ten7 development environments exist to test not only code but code and content. For paying clients, time spent waiting for a larger site to copy files and database every build is costly especially when we have to restore a working db or files with sample content for demonstration purposes. We either have to absorb the costs in development time or we have to pass them on to the client.
We need the exact repeatability only when on the path to release to live. As a developer, I'm aware of the risks of developing with a stale database, but that should be on me to fix. Especially since I'm often restoring a modified database after a deployment to get back to the state I need the site for client or colleague review.
Thanks for your feedback :+1: Do you have any numbers around the time it's taking to copy a database/set of files of x size?
For use case 2. we'd tend to use https://www.drupal.org/project/default_content/ and then remove it if necessary before merge
On a moderately large site (say one with a 50MB or larger DB, it takes as much as 10 additional minutes to restore a database after a build (which also takes about 3-5 more minutes per build to copy a new db. So in the end we're spending as much as 20 minutes on a build and we can't step away from the work because we need to reload the db so we can continue to show a client the changes we're proposing. This is most commonly an issue when we're working on a large epic that involves, say, a new functionality on the site.
@justafish Here's a patch we use on one of our sites. It could just be a patch here that could be offered until some option could be to incorporate. You have a marvelous product and I understand this is a product mainly designed for Lullabot's workflows, but this could be a useful option (possibly even in your own shop) drainpipe-skip-wipe-357.patch
This feature would be a useful option for our support clients as well. We recently had a project where we enabled this feature but the client told us to disable it because of how many GitHub Actions minutes were being used. cc @YesCT @DBNubs
Yeah, we need the job to be super fast for adding the job to the customer that is sensitive to their GitHub minutes usage. Looping in @leonel-lullabot also who is doing some investigation on where the time is being spent in the PantheonReviewApps / multidev deploys.