Skip to content

Commit

Permalink
Migrate build scripts to python with single script to orchestrate dur…
Browse files Browse the repository at this point in the history
…ing make up (#23011)

* Migrate build scripts to python with single script to orchestrate during make up
- Replaced the locale compilation script from a shell script to a Python script (compile_locales.py) for better error handling and parallel processing.
- Updated the update_assets target in Makefile-docker to use the new update_assets.py script.
- Removed the obsolete compile-mo.sh script.
- Introduced sync_host_files.py to streamline dependency updates and asset synchronization.

* TMP: add tests
  • Loading branch information
KevinMind authored Jan 27, 2025
1 parent 7c3d1ea commit 6cc53d9
Show file tree
Hide file tree
Showing 11 changed files with 354 additions and 54 deletions.
9 changes: 6 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ EOF

FROM base AS development

# Copy build info from info
COPY --from=info ${BUILD_INFO} ${BUILD_INFO}

FROM base AS locales
ARG LOCALE_DIR=${HOME}/locale
# Compile locales
Expand All @@ -155,7 +158,7 @@ COPY --chown=olympia:olympia locale ${LOCALE_DIR}
RUN \
--mount=type=bind,source=requirements/locale.txt,target=${HOME}/requirements/locale.txt \
--mount=type=bind,source=Makefile-docker,target=${HOME}/Makefile-docker \
--mount=type=bind,source=locale/compile-mo.sh,target=${HOME}/compile-mo.sh \
--mount=type=bind,source=scripts/compile_locales.py,target=${HOME}/scripts/compile_locales.py \
make -f Makefile-docker compile_locales

# More efficient caching by mounting the exact files we need
Expand All @@ -175,10 +178,10 @@ COPY --chown=olympia:olympia static/ ${HOME}/static/
RUN \
--mount=type=bind,src=src,target=${HOME}/src \
--mount=type=bind,src=Makefile-docker,target=${HOME}/Makefile-docker \
--mount=type=bind,src=scripts/update_assets.py,target=${HOME}/scripts/update_assets.py \
--mount=type=bind,src=manage.py,target=${HOME}/manage.py \
<<EOF
echo "from olympia.lib.settings_base import *" > settings_local.py
DJANGO_SETTINGS_MODULE="settings_local" make -f Makefile-docker update_assets
make -f Makefile-docker update_assets
EOF

FROM base AS production
Expand Down
12 changes: 3 additions & 9 deletions Makefile-docker
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,8 @@ data_load:
./manage.py data_load $(ARGS)

.PHONY: update_assets
update_assets:
# Copy files required in compress_assets to the static folder
# If changing this here, make sure to adapt tests in amo/test_commands.py
$(PYTHON_COMMAND) manage.py compress_assets
$(PYTHON_COMMAND) manage.py generate_jsi18n_files
# Collect static files: This MUST be run last or files will be missing
$(PYTHON_COMMAND) manage.py collectstatic --noinput

update_assets: ## Update the static assets
$(HOME)/scripts/update_assets.py

.PHONY: update_deps
update_deps: ## Update the dependencies
Expand Down Expand Up @@ -218,7 +212,7 @@ extract_locales: ## extracts and merges translation strings
.PHONE: compile_locales
compile_locales: ## compiles translation strings
$(PIP_COMMAND) install --progress-bar=off --no-deps -r requirements/locale.txt
./locale/compile-mo.sh ./locale/
$(HOME)/scripts/compile_locales.py

.PHONY: help_submake
help_submake:
Expand Down
8 changes: 4 additions & 4 deletions Makefile-os
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,17 @@ docker_clean_build_cache: ## Remove buildx build cache
.PHONY: clean_docker
clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_images docker_clean_volumes docker_clean_build_cache ## Remove all docker resources taking space on the host machine

.PHONY: docker_update_deps
docker_update_deps: docker_mysqld_volume_create ## Update the dependencies in the container based on the docker tag and target
.PHONY: docker_sync_host
docker_sync_host: docker_mysqld_volume_create ## Update the dependencies in the container based on the docker tag and target
docker compose run \
--rm \
--no-deps \
$(DOCKER_RUN_ARGS) \
web \
make update_deps
./scripts/sync_host_files.py

.PHONY: up_pre
up_pre: setup docker_pull_or_build docker_update_deps ## Pre-up the environment, setup files, volumes and host state
up_pre: setup docker_pull_or_build docker_sync_host ## Pre-up the environment, setup files, volumes and host state

.PHONY: up_start
up_start: docker_mysqld_volume_create ## Start the docker containers
Expand Down
37 changes: 0 additions & 37 deletions locale/compile-mo.sh

This file was deleted.

60 changes: 60 additions & 0 deletions scripts/compile_locales.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env python3

import os
import subprocess
from concurrent.futures import ThreadPoolExecutor
from pathlib import Path


def process_po_file(pofile, attempt=1):
"""Process a single .po file, creating corresponding .mo file."""
pofile_path = Path(pofile)
print('processing', pofile_path.as_posix())
mo_path = pofile_path.with_suffix('.mo')

mo_path.touch()

try:
# Run dennis-cmd lint
subprocess.run(
['dennis-cmd', 'lint', '--errorsonly', pofile],
capture_output=True,
check=False,
)
# If lint passes, run msgfmt
subprocess.run(['msgfmt', '-o', mo_path, pofile], check=True)
return
except subprocess.CalledProcessError as e:
if attempt < 3:
print(f'Failed attempt {attempt} for {pofile}, retrying...')
return process_po_file(pofile, attempt=attempt + 1)
raise e


def compile_locales():
# Ensure 'dennis' is installed
import dennis as _dennis # type: ignore # noqa: F401

HOME = os.environ.get('HOME')

locale_dir = Path(HOME) / 'locale'

print(f'Compiling locales in {locale_dir}')

# Collect all files first
django_files = []
djangojs_files = []
for root, _, files in locale_dir.walk():
for file in files:
if file == 'django.po':
django_files.append(root / file)
elif file == 'djangojs.po':
djangojs_files.append(root / file)

# Process django.po files in parallel
with ThreadPoolExecutor() as executor:
executor.map(process_po_file, django_files + djangojs_files)


if __name__ == '__main__':
compile_locales()
22 changes: 22 additions & 0 deletions scripts/sync_host_files.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/usr/bin/env python3

import json
import os
import subprocess


def sync_host_files():
BUILD_INFO = os.environ.get('BUILD_INFO')

subprocess.run(['make', 'update_deps'], check=True)

with open(BUILD_INFO, 'r') as f:
build_info = json.load(f)

if build_info.get('target') == 'production':
subprocess.run(['make', 'compile_locales'], check=True)
subprocess.run(['make', 'update_assets'], check=True)


if __name__ == '__main__':
sync_host_files()
57 changes: 57 additions & 0 deletions scripts/update_assets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env python3

import argparse
import os
import shutil
import subprocess
from pathlib import Path


def clean_static_dirs(verbose: bool = False):
HOME = os.environ.get('HOME')
STATIC_DIRS = ['static-build', 'site-static']

for directory in STATIC_DIRS:
path = Path(HOME) / directory
path.mkdir(parents=True, exist_ok=True)
for entry in path.iterdir():
entry_path = entry.as_posix()
if verbose:
print(f'Removing {entry_path}')
if entry.is_dir():
shutil.rmtree(entry_path)
else:
os.remove(entry_path)


def update_assets(verbose: bool = False):
clean_static_dirs(verbose)

script_prefix = ['python3', 'manage.py']

environment = os.environ.copy()
# Always run in production mode without any development settings
environment['DJANGO_SETTINGS_MODULE'] = 'olympia.lib.settings_base'

subprocess.run(
script_prefix + ['compress_assets'],
check=True,
env=environment,
)
subprocess.run(
script_prefix + ['generate_jsi18n_files'],
check=True,
env=environment,
)
subprocess.run(
script_prefix + ['collectstatic', '--noinput'],
check=True,
env=environment,
)


if __name__ == '__main__':
parser = argparse.ArgumentParser()
parser.add_argument('--verbose', action='store_true')
args = parser.parse_args()
update_assets(args.verbose)
2 changes: 1 addition & 1 deletion src/olympia/lib/settings_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,7 @@ def read_only_mode(env):
NODE_PACKAGE_JSON = os.path.join('/', 'deps', 'package.json')
NODE_PACKAGE_MANAGER_INSTALL_OPTIONS = ['--dry-run']

STATIC_BUILD_PATH = os.path.join('/', 'data', 'olympia', 'static-build')
STATIC_BUILD_PATH = path('static-build')

STATICFILES_DIRS = (
path('static'),
Expand Down
89 changes: 89 additions & 0 deletions tests/make/test_compile_locales.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import subprocess
import sys
import tempfile
from pathlib import Path
from unittest import TestCase, mock
from unittest.mock import Mock, patch

import pytest

from scripts.compile_locales import compile_locales, process_po_file
from tests import override_env


@pytest.mark.needs_locales_compilation
class TestCompileLocales(TestCase):
def setUp(self):
self.home_dir = Path(tempfile.mkdtemp())
self.locale_dir = (self.home_dir / 'locale').mkdir()

@patch.dict(sys.modules, {'dennis': None})
def test_dennis_not_installed(self):
"""Test that the script raises when dennis is not installed"""
self.assertRaises(ImportError, compile_locales)

@patch.dict(sys.modules, {'dennis': Mock()})
@patch('scripts.compile_locales.ThreadPoolExecutor')
def test_process_po_file(self, mock_executor):
"""Test that the script processes po files"""
# Create po files
django_po = self.home_dir / 'locale' / 'django.po'
django_po.touch()
djangojs_po = self.home_dir / 'locale' / 'djangojs.po'
djangojs_po.touch()

# Setup ThreadPoolExecutor mock
mock_executor_instance = Mock()
mock_executor.return_value.__enter__.return_value = mock_executor_instance

with override_env(HOME=self.home_dir.as_posix()):
compile_locales()

# Get the actual arguments passed to map
actual_args = mock_executor_instance.map.call_args[0]
self.assertEqual(actual_args[0], process_po_file)
self.assertEqual(list(actual_args[1]), [django_po, djangojs_po])


class TestProcessPoFile(TestCase):
def setUp(self):
self.pofile = Path(tempfile.mkdtemp()) / 'django.po'

mock_subprocess = patch('scripts.compile_locales.subprocess.run')
self.mock_subprocess = mock_subprocess.start()
self.addCleanup(mock_subprocess.stop)

def test_process_po_file(self):
process_po_file(self.pofile.as_posix())
self.assertTrue(self.pofile.with_suffix('.mo').exists())

assert self.mock_subprocess.call_args_list == [
mock.call(
['dennis-cmd', 'lint', '--errorsonly', self.pofile.as_posix()],
capture_output=True,
check=False,
),
mock.call(
[
'msgfmt',
'-o',
self.pofile.with_suffix('.mo'),
self.pofile.as_posix(),
],
check=True,
),
]

def test_process_po_file_retries(self):
self.mock_subprocess.side_effect = subprocess.CalledProcessError(
returncode=1,
cmd=['dennis-cmd', 'lint', '--errorsonly', self.pofile.as_posix()],
)

with self.assertRaises(subprocess.CalledProcessError):
process_po_file(self.pofile.as_posix())

self.assertTrue(self.pofile.with_suffix('.mo').exists())

# We expect 3 attempts to process the file
self.assertEqual(self.mock_subprocess.call_count, 3)
36 changes: 36 additions & 0 deletions tests/make/test_sync_host_files.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import json
import tempfile
from pathlib import Path
from unittest import TestCase, mock

from scripts.sync_host_files import sync_host_files
from tests import override_env


@mock.patch('scripts.sync_host_files.subprocess.run')
class TestSyncHostFiles(TestCase):
def test_sync_host_files(self, mock_subprocess):
sync_host_files()

mock_subprocess.assert_has_calls(
[
mock.call(['make', 'update_deps'], check=True),
# mock.call(['make', 'compile_locales'], check=True),
# mock.call(['make', 'update_assets'], check=True),
]
)

def test_sync_host_files_production(self, mock_subprocess):
mock_build = Path(tempfile.mktemp())
mock_build.write_text(json.dumps({'target': 'production'}))

with override_env(BUILD_INFO=mock_build.as_posix()):
sync_host_files()

mock_subprocess.assert_has_calls(
[
mock.call(['make', 'update_deps'], check=True),
mock.call(['make', 'compile_locales'], check=True),
mock.call(['make', 'update_assets'], check=True),
]
)
Loading

0 comments on commit 6cc53d9

Please sign in to comment.