drainpipe icon indicating copy to clipboard operation
drainpipe copied to clipboard

Make wipe optional in Pantheon review action

Open tess-ten7 opened this issue 2 years ago • 14 comments

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 avatar Dec 19 '23 22:12 tess-ten7

@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 avatar Jan 22 '24 23:01 justafish

@justafish There are two use cases which come to mind:

  1. The upstream database/files are rather large, drastically increasing our Github action costs while those sync.
  2. 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.

tess-ten7 avatar Apr 02 '24 17:04 tess-ten7

@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.

generalludd avatar Jun 04 '24 17:06 generalludd

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

justafish avatar Jun 05 '24 09:06 justafish

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.

generalludd avatar Jun 21 '24 21:06 generalludd

@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

generalludd avatar Jun 24 '24 14:06 generalludd

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

mrdavidburns avatar Jun 24 '24 18:06 mrdavidburns

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.

YesCT avatar Jun 24 '24 18:06 YesCT