Developer experience improvements around DB downloads and getting CMS…

… images (#15079)

* Update "make preflight" command to allow a -- --retain-db flag

This will help people avoid blatting their local DB and losing WIP CMS pages

* Improve formatting (line breaks) for the boostrap_local_admin management command

* Add tool to download images to match a fresh DB download

* Expand set to default rendition sizes exported, so that downloaded images all have appropriate sizes made

The 165x165 is what the Image Library in Wagtail needs, so we must ensure a local download of images regenerates that immediately

* Add docs for the image-download tool

* Exclude image renditions from the DB export

These are not needed, and indeed block the [re]generation of renditions after download. If a DB row exists for a rendition, even though the image does not, Wagtail won't regenerate an image rendition to replace it because it thinks it already exists.

* Test fixups

* Add extra line to Makefile help explain how to use --retain-db flag via make

* Add docs explaining how to get CMS state down to local dev

* Minor fixups following code review

Makefile

@@ -36,6 +36,7 @@ help:
 	@echo "  uninstall-custom-git-hooks     - uninstall custom git hooks"
 	@echo "  clean-local-deps               - remove all local installed Python dependencies"
 	@echo "  preflight                      - refresh installed dependencies and fetch latest DB ahead of local dev"
+	@echo "  preflight -- --retain-DB		- refresh installed dependencies WITHOUT fetching latest DB"
 	@echo "  run-local-task-queue           - run rqworker on your local machine. Requires redis to be running"
 
 .env:
@@ -169,19 +170,23 @@ check-requirements: .docker-build-pull
 # For use in local-machine development (not in Docker)
 ######################################################
 
+# Trick to avoid treating flags (eg --retain-db) as a make target
+%:
+	@:
+
+preflight:
+	${MAKE} install-local-python-deps
+	@npm install
+	@$(if $(findstring --retain-db,$(MAKECMDGOALS)),bin/sync-all.sh --retain-db,bin/sync-all.sh)
+	@python manage.py bootstrap_local_admin
+
 install-local-python-deps:
 	# Dev requirements are a superset of prod requirements, but we install
 	# them in the same separate steps that we use for our Docker-based build,
 	# so that it mirrors Production and Dev image building
 	pip install -r requirements/prod.txt
 	pip install -r requirements/dev.txt
 
-preflight:
-	${MAKE} install-local-python-deps
-	$ npm install
-	$ bin/sync-all.sh
-	$ python manage.py bootstrap_local_admin
-
 run-local-task-queue:
 	# We temporarily source the .env for the command's duration only
 	(source ".env" && \

bedrock/cms/management/commands/bootstrap_local_admin.py

@@ -24,23 +24,23 @@ def handle(self, *args, **kwargs):
         WAGTAIL_ADMIN_PASSWORD = config("WAGTAIL_ADMIN_PASSWORD", default="")
 
         if not WAGTAIL_ADMIN_EMAIL:
-            sys.stdout.write("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment.")
+            sys.stdout.write("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment\n")
             return
         if not WAGTAIL_ADMIN_EMAIL.endswith("@mozilla.com"):
-            sys.stdout.write("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL is not a @mozilla.com email address.")
+            sys.stdout.write("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL is not a @mozilla.com email address\n")
             return
 
         user, created = User.objects.get_or_create(email=WAGTAIL_ADMIN_EMAIL)
         if not created:
-            sys.stdout.write(f"Admin user {WAGTAIL_ADMIN_EMAIL} already exists")
+            sys.stdout.write(f"Admin user {WAGTAIL_ADMIN_EMAIL} already exists\n")
         else:
             user.username = WAGTAIL_ADMIN_EMAIL
             user.is_staff = True
             user.is_superuser = True
             if not WAGTAIL_ADMIN_PASSWORD:
                 user.set_unusable_password()  # They won't need one to use SSO
-                sys.stdout.write(f"Created Admin user {WAGTAIL_ADMIN_EMAIL} for local SSO use")
+                sys.stdout.write(f"Created Admin user {WAGTAIL_ADMIN_EMAIL} for local SSO use\n")
             else:
                 user.set_password(WAGTAIL_ADMIN_PASSWORD)
-                sys.stdout.write(f"Created Admin user {WAGTAIL_ADMIN_EMAIL} with password '{WAGTAIL_ADMIN_PASSWORD}'")
+                sys.stdout.write(f"Created Admin user {WAGTAIL_ADMIN_EMAIL} with password '{WAGTAIL_ADMIN_PASSWORD}'\n")
             user.save()

bedrock/cms/management/commands/download_media_to_local.py

@@ -0,0 +1,77 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+import os
+import sys
+
+from django.conf import settings
+from django.core.management.base import BaseCommand
+
+from google.cloud import storage
+
+from bedrock.cms.models import BedrockImage
+from bedrock.settings.base import path as build_path
+
+BUCKETS = {
+    "dev": "bedrock-nonprod-dev-cms-media",
+    "stage": "bedrock-nonprod-stage-cms-media",
+    "prod": "bedrock-prod-prod-cms-media",
+}
+
+
+class Command(BaseCommand):
+    help = """Downloads public media files from the appropriate cloud bucket
+    so that they match the sqlite database being currently used."""
+
+    def add_arguments(self, parser):
+        parser.add_argument(
+            "--environment",
+            default="dev",
+            help="Which Bedrock environment are you downloading from? Values are dev | stage | prod (default is dev)",
+        )
+        parser.add_argument(
+            "--redownload",
+            action="store_true",
+            dest="redownload",
+            default=False,
+            help="If passed, will force re-download of all the assets in the relevant bucket.",
+        )
+
+    def handle(self, *args, **options):
+        # If we're not using sqlite, stop, because this tool isn't for that
+
+        if settings.DATABASES["default"]["ENGINE"] != "django.db.backends.sqlite3":
+            self.stderr.write(f"This command only works if you are using sqlite as your local DB. Got {settings.DATABASES['default']['ENGINE']}\n")
+            sys.exit(1)
+
+        try:
+            bucket_name = BUCKETS[options["environment"]]
+        except KeyError:
+            self.stderr.write(f"Couldn't determine which bucket you wanted. Got {options['environment']}\n")
+            sys.exit(1)
+
+        storage_client = storage.Client.create_anonymous_client()
+        bucket = storage_client.bucket(bucket_name)
+
+        # Get the files, ideally in a way that checks whether we have them already
+        # unless the --redownload param is passed
+        redownload = options["redownload"]
+        if redownload:
+            self.stdout.write("Forcing redownload of all files.\n")
+
+        for image in BedrockImage.objects.all():
+            image_key = f"media/cms/{image.file.name}"
+            local_dest = build_path(settings.MEDIA_ROOT, image.file.name)
+
+            if os.path.exists(local_dest) and not redownload:
+                self.stdout.write(f"Skipping: {local_dest} already exists locally.\n")
+            else:
+                blob = bucket.blob(image_key)
+                blob.download_to_filename(local_dest)
+                self.stdout.write(f"Downloaded {image_key} from {bucket_name} to {local_dest}\n")
+
+                image._pre_generate_expected_renditions()
+                self.stdout.write("Triggered local generation of renditions\n")
+
+        self.stdout.write("All done.\n")

bedrock/cms/models/images.py

@@ -8,7 +8,18 @@
 
 from bedrock.base.tasks import defer_task
 
-AUTOMATIC_RENDITION_FILTER_SPECS = [f"width-{size}" for size in range(2400, 0, -200)] + ["width-100"]
+AUTOMATIC_RENDITION_FILTER_SPECS = [
+    # Generate agreed step sizes:
+    f"width-{size}"
+    for size in range(2400, 0, -200)
+] + [
+    # Add agreed smallest size
+    "width-100",
+    # Add the size that the Wagtail Image Library uses for its listing page - we make sure we
+    # regenerate it, if needed, so that the image library is updated locally after
+    # downloading images from Dev, Stage or Prod
+    "max-165x165",
+]
 
 
 class BedrockImage(AbstractImage):

bedrock/cms/test_image_models.py

@@ -27,6 +27,7 @@ def test_pre_generate_expected_renditions__no_queue_available(self):
             "width-400",
             "width-200",
             "width-100",
+            "max-165x165",
         ]
 
         with patch.object(image, "get_renditions") as get_renditions_mock:
@@ -50,6 +51,7 @@ def test_pre_generate_expected_renditions__queue_available(self):
             "width-400",
             "width-200",
             "width-100",
+            "max-165x165",
         ]
 
         with patch("bedrock.base.tasks.django_rq") as mock_django_rq:
@@ -80,6 +82,7 @@ def test_pre_generate_expected_renditions_uses_defer_task(self):
             "width-400",
             "width-200",
             "width-100",
+            "max-165x165",
         ]
         with patch("bedrock.cms.models.images.defer_task") as mock_defer_task:
             image._pre_generate_expected_renditions()

bedrock/cms/tests/test_commands.py

@@ -22,7 +22,7 @@ def test_no_env_vars_available(self, mock_write):
         self._run_test(
             mock_write=mock_write,
             expected_output=[
-                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment."),
+                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment\n"),
             ],
         )
 
@@ -31,7 +31,7 @@ def test_email_available(self, mock_write):
         self._run_test(
             mock_write=mock_write,
             expected_output=[
-                call("Created Admin user [email protected] for local SSO use"),
+                call("Created Admin user [email protected] for local SSO use\n"),
             ],
         )
 
@@ -40,7 +40,7 @@ def test_email_available_but_not_moco(self, mock_write):
         self._run_test(
             mock_write=mock_write,
             expected_output=[
-                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL is not a @mozilla.com email address."),
+                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL is not a @mozilla.com email address\n"),
             ],
         )
 
@@ -49,7 +49,7 @@ def test_email_and_password_available(self, mock_write):
         self._run_test(
             mock_write=mock_write,
             expected_output=[
-                call("Created Admin user [email protected] with password 'secret'"),
+                call("Created Admin user [email protected] with password 'secret'\n"),
             ],
         )
 
@@ -58,7 +58,7 @@ def test_only_password_available(self, mock_write):
         self._run_test(
             mock_write=mock_write,
             expected_output=[
-                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment."),
+                call("Not bootstrapping an Admin user: WAGTAIL_ADMIN_EMAIL not defined in environment\n"),
             ],
         )
 
@@ -69,8 +69,8 @@ def test_existing_user_exists_email_only(self, mock_write):
         call_command("bootstrap_local_admin", stdout=out)
         output = mock_write.call_args_list
         expected_output = [
-            call("Created Admin user [email protected] for local SSO use"),
-            call("Admin user [email protected] already exists"),
+            call("Created Admin user [email protected] for local SSO use\n"),
+            call("Admin user [email protected] already exists\n"),
         ]
         self.assertEqual(output, expected_output)
 
@@ -81,7 +81,7 @@ def test_existing_user_exists_email_and_password(self, mock_write):
         call_command("bootstrap_local_admin", stdout=out)
         output = mock_write.call_args_list
         expected_output = [
-            call("Created Admin user [email protected] with password 'secret'"),
-            call("Admin user [email protected] already exists"),
+            call("Created Admin user [email protected] with password 'secret'\n"),
+            call("Admin user [email protected] already exists\n"),
         ]
         self.assertEqual(output, expected_output)

bin/custom-git-hooks/post-merge

@@ -15,7 +15,7 @@ def check_for_dependency_changes():
             WARNING,
             "Dependency files have changed! Remember to install new dependencies with",
             COMMAND,
-            "make preflight",
+            "make preflight -- --retain-db",
             RESET,
         )
 

bin/export-db-to-sqlite.sh

@@ -156,7 +156,6 @@ python manage.py dumpdata \
     cms.StructuralPage \
     cms.SimpleRichTextPage \
     cms.BedrockImage \
-    cms.BedrockRendition \
     legal_docs.LegalDoc \
     mozorg.WebvisionDoc \
     mozorg.LeadershipPage \

bin/sync-all.sh

@@ -10,6 +10,20 @@ if [ ! -e ./manage.py ]; then
     cd $script_parent_dir
 fi
 
-./bin/run-db-download.py --force
+# Check whether --retain-db was passed
+DO_DB_DOWNLOAD=true
+for arg in "$@"; do
+  if [ "$arg" == "--retain-db" ]; then
+    DO_DB_DOWNLOAD=false
+    break
+  fi
+done
+
+if [ "$DO_DB_DOWNLOAD" = true ]; then
+    ./bin/run-db-download.py --force
+else
+    echo "Skipping DB download"
+fi
+
 ./manage.py migrate --noinput
 ./manage.py l10n_update