From 354a7cfb4ff130ac722dad87ff2bf1369e68c7d8 Mon Sep 17 00:00:00 2001 From: Zeid Zabaneh Date: Mon, 1 Apr 2024 14:17:26 -0400 Subject: [PATCH] testing: fix up various aspects that are breaking tests (bug 1887042) WIP DO NOT MERGE - fix ups to get tests to run - remove flask celery functionality - add boilerplate django celery impl - add db to github workflow --- .github/workflows/build.yml | 18 ++- pyproject.toml | 3 + requirements.txt | 9 ++ src/lando/api/legacy/app.py | 1 - src/lando/api/legacy/celery.py | 113 ++---------------- src/lando/api/legacy/cli.py | 15 +-- src/lando/api/legacy/mocks/auth.py | 2 +- src/lando/api/legacy/projects.py | 7 +- src/lando/api/legacy/uplift.py | 9 +- .../api/legacy/workers/landing_worker.py | 28 ++--- src/lando/api/tests/conftest.py | 77 +++--------- src/lando/api/tests/mocks.py | 2 +- src/lando/api/tests/test_decorators.py | 2 +- src/lando/api/tests/test_dockerflow.py | 2 +- src/lando/api/tests/test_health.py | 50 -------- src/lando/api/tests/test_landing_job.py | 12 +- src/lando/api/tests/test_landings.py | 28 ++--- src/lando/api/tests/test_notifications.py | 6 +- .../api/tests/test_phabricator_client.py | 2 +- src/lando/api/tests/test_py3_codequality.py | 26 ---- src/lando/api/tests/test_transplants.py | 40 +++---- src/lando/{jinja2.py => jinja.py} | 0 src/lando/main/admin.py | 4 +- src/lando/main/migrations/0001_initial.py | 3 +- .../migrations/0002_configurationvariable.py | 48 ++++++++ src/lando/main/models/__init__.py | 4 +- src/lando/main/models/configuration.py | 2 +- src/lando/main/models/landing_job.py | 6 +- src/lando/main/models/revision.py | 2 +- src/lando/main/support.py | 4 +- src/lando/settings.py | 3 +- 31 files changed, 184 insertions(+), 344 deletions(-) delete mode 100644 src/lando/api/tests/test_health.py delete mode 100644 src/lando/api/tests/test_py3_codequality.py rename src/lando/{jinja2.py => jinja.py} (100%) create mode 100644 src/lando/main/migrations/0002_configurationvariable.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f0227e83..55885158 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,10 +8,25 @@ on: jobs: run-tests: runs-on: ubuntu-latest + services: + postgres: + image: postgres:latest + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 steps: - uses: actions/checkout@v4 - name: Set up Python uses: actions/setup-python@v4 + env: + DEFAULT_DB_HOST: 127.0.0.1 with: python-version: "3.10" - name: Install dependencies @@ -24,5 +39,6 @@ jobs: - name: Test run: | source env/bin/activate + lando migrate lando test - pytest + pytest src/lando/api diff --git a/pyproject.toml b/pyproject.toml index 6cce69dc..efbcc273 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,3 +32,6 @@ build-backend = "setuptools.build_meta" [tool.setuptools.packages.find] where = ["src"] + +[tool.pytest.ini_options] +DJANGO_SETTINGS_MODULE = "lando.settings" diff --git a/requirements.txt b/requirements.txt index 36799a50..681c79e2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,15 @@ black jinja2 +kombu pytest pytest-django +python-hglib +python-jose +redis +requests-mock ruff uwsgi +celery +datadog +rs-parsepatch +networkx diff --git a/src/lando/api/legacy/app.py b/src/lando/api/legacy/app.py index d3f5a30f..ea1d3fcb 100644 --- a/src/lando/api/legacy/app.py +++ b/src/lando/api/legacy/app.py @@ -9,7 +9,6 @@ import connexion from connexion.resolver import RestyResolver -import landoapi.models # noqa, makes sure alembic knows about the models. from lando.api.legacy.auth import auth0_subsystem from lando.api.legacy.cache import cache_subsystem from lando.api.legacy.celery import celery_subsystem diff --git a/src/lando/api/legacy/celery.py b/src/lando/api/legacy/celery.py index 996eb3bc..c74feda6 100644 --- a/src/lando/api/legacy/celery.py +++ b/src/lando/api/legacy/celery.py @@ -1,9 +1,9 @@ # 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 http://mozilla.org/MPL/2.0/. +import os import logging -import flask from celery import Celery from celery.signals import ( after_task_publish, @@ -15,87 +15,24 @@ ) from datadog import statsd -from lando.api.legacy.systems import Subsystem - logger = logging.getLogger(__name__) -class FlaskCelery(Celery): - """Celery which executes task in a flask app context.""" - - def __init__(self, *args, **kwargs): - # Avoid passing the flask app to base Celery. - flask_app = kwargs.pop("app", None) - - super().__init__(*args, **kwargs) - - # Important to run this after __init__ since task_cls - # argument to base Celery can change what we're basing on. - self._flask_override_task_class() - - if flask_app is not None: - self.init_app(flask_app) - - @property - def dispatch_disabled(self): - """Will the Celery job system dispatch tasks to the workers?""" - return bool(self.app.config.get("DISABLE_CELERY")) - - def init_app(self, app, config=None): - """Initialize with a flask app.""" - self.app = app - - config = config or {} - self.conf.update(main=app.import_name, **config) - - if self.dispatch_disabled: - logger.warning( - "DISABLE_CELERY application configuration variable set, the Celery job " - "system has been disabled! Any features that depend on the job system " - "will not function." - ) - - def _flask_override_task_class(self): - """Change Task class to one which executes in a flask context.""" - # Define a Task subclass that saves a reference to self in the Task object so - # the task object can find self.app (the Flask application object) even if - # self.app hasn't been set yet. - # - # We need to delay all of the task's calls to self.app using a custom Task class - # because the reference to self.app may not be valid at the time the Celery - # application object creates it set of Task objects. The programmer may - # set self.app via the self.init_app() method at any time in the future. - # - # self.app is expected to be valid and usable by Task objects after the web - # application is fully initialized and ready to serve requests. - BaseTask = self.Task - celery_self = self - - class FlaskTask(BaseTask): - """A Celery Task subclass that has a reference to a Flask app.""" +# Set the default Django settings module for the 'celery' program. +os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'lando.settings') - def __call__(self, *args, **kwargs): - # Override immediate calling of tasks, such as mytask(). This call - # method is used by the Celery worker process. - if flask.has_app_context(): - return super().__call__(*args, **kwargs) +app = Celery('lando') - with celery_self.app.app_context(): - return super().__call__(*args, **kwargs) +# Using a string here means the worker doesn't have to serialize +# the configuration object to child processes. +# - namespace='CELERY' means all celery-related configuration keys +# should have a `CELERY_` prefix. +app.config_from_object('django.conf:settings', namespace='CELERY') - def apply_async(self, *args, **kwargs): - # Override delayed calling of tasks, such as mytask.apply_async(). - # This call method is used by the Celery app when it wants to - # schedule a job for eventual execution on a worker. - if celery_self.dispatch_disabled: - return None - else: - return super().apply_async(*args, **kwargs) +# Load task modules from all registered Django apps. +app.autodiscover_tasks() - self.Task = FlaskTask - - -celery = FlaskCelery() +celery = app @after_task_publish.connect @@ -129,29 +66,3 @@ def count_task_retried(**kwargs): def setup_celery_logging(**kwargs): # Prevent celery from overriding our logging configuration. pass - - -class CelerySubsystem(Subsystem): - name = "celery" - - def init_app(self, app): - super().init_app(app) - - # Import tasks to discover celery tasks. - import landoapi.tasks # noqa - - celery.init_app( - self.flask_app, - config={"broker_url": settings.CELERY_BROKER_URL}, - ) - celery.log.setup() - - def ready(self): - if settings.DISABLE_CELERY: - return True - - # TODO: Check connection to CELERY_BROKER_URL - return True - - -celery_subsystem = CelerySubsystem() diff --git a/src/lando/api/legacy/cli.py b/src/lando/api/legacy/cli.py index 2064d6f1..948ca0f6 100644 --- a/src/lando/api/legacy/cli.py +++ b/src/lando/api/legacy/cli.py @@ -8,13 +8,11 @@ from typing import Optional import click -import connexion -from flask.cli import FlaskGroup from lando.main.models.configuration import ( ConfigurationKey, ConfigurationVariable, - VariableType, + VariableTypeChoices, ) from lando.api.legacy.systems import Subsystem @@ -35,7 +33,7 @@ def get_subsystems(exclude: Optional[list[Subsystem]] = None) -> list[Subsystem] return [s for s in SUBSYSTEMS if s not in exclusions] -def create_lando_api_app() -> connexion.App: +def create_lando_api_app(): from lando.api.legacy.app import construct_app, load_config config = load_config() @@ -46,7 +44,6 @@ def create_lando_api_app() -> connexion.App: return app.app -@click.group(cls=FlaskGroup, create_app=create_lando_api_app) def cli(): """Lando API cli.""" @@ -83,10 +80,10 @@ def landing_worker(): def run_pre_deploy_sequence(): """Runs the sequence of commands required before a deployment.""" ConfigurationVariable.set( - ConfigurationKey.API_IN_MAINTENANCE, VariableType.BOOL, "1" + ConfigurationKey.API_IN_MAINTENANCE, VariableTypeChoices.BOOL, "1" ) ConfigurationVariable.set( - ConfigurationKey.LANDING_WORKER_PAUSED, VariableType.BOOL, "1" + ConfigurationKey.LANDING_WORKER_PAUSED, VariableTypeChoices.BOOL, "1" ) @@ -94,10 +91,10 @@ def run_pre_deploy_sequence(): def run_post_deploy_sequence(): """Runs the sequence of commands required after a deployment.""" ConfigurationVariable.set( - ConfigurationKey.API_IN_MAINTENANCE, VariableType.BOOL, "0" + ConfigurationKey.API_IN_MAINTENANCE, VariableTypeChoices.BOOL, "0" ) ConfigurationVariable.set( - ConfigurationKey.LANDING_WORKER_PAUSED, VariableType.BOOL, "0" + ConfigurationKey.LANDING_WORKER_PAUSED, VariableTypeChoices.BOOL, "0" ) diff --git a/src/lando/api/legacy/mocks/auth.py b/src/lando/api/legacy/mocks/auth.py index fabda3a0..1844b1bb 100644 --- a/src/lando/api/legacy/mocks/auth.py +++ b/src/lando/api/legacy/mocks/auth.py @@ -90,4 +90,4 @@ def access_token(self): @property def mock_headers(self): - return [("Authorization", "Bearer {}".format(self.access_token))] + return dict([("Authorization", "Bearer {}".format(self.access_token))]) diff --git a/src/lando/api/legacy/projects.py b/src/lando/api/legacy/projects.py index bc32e1b9..8f96c648 100644 --- a/src/lando/api/legacy/projects.py +++ b/src/lando/api/legacy/projects.py @@ -4,7 +4,8 @@ import logging from typing import Optional -from lando.api.legacy.cache import DEFAULT_CACHE_KEY_TIMEOUT_SECONDS, cache +from django.core.cache import cache + from lando.api.legacy.phabricator import PhabricatorClient, result_list_to_phid_dict logger = logging.getLogger(__name__) @@ -57,7 +58,7 @@ def project_search( "project.search", constraints={"phids": project_phids} ) result = result_list_to_phid_dict(phabricator.expect(projects, "data")) - cache.set(cache_key, result, timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS) + cache.set(cache_key, result) return result @@ -92,7 +93,7 @@ def get_project_phid( ) value = phabricator.expect(project, "phid") if project else None - cache.set(key, value, timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS) + cache.set(key, value) return value diff --git a/src/lando/api/legacy/uplift.py b/src/lando/api/legacy/uplift.py index ab8f264e..0d7b728c 100644 --- a/src/lando/api/legacy/uplift.py +++ b/src/lando/api/legacy/uplift.py @@ -18,8 +18,7 @@ Version, ) -from lando.api import bmo -from lando.api.legacy.cache import DEFAULT_CACHE_KEY_TIMEOUT_SECONDS, cache +from lando.api.legacy import bmo from lando.api.legacy.phabricator import PhabricatorClient from lando.api.legacy.phabricator_patch import patch_to_changes from lando.api.legacy.repos import ( @@ -68,9 +67,9 @@ def get_uplift_request_form(revision: dict) -> Optional[str]: return bug -@cache.cached( - key_prefix="uplift-repositories", timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS -) +#@cache.cached( +# key_prefix="uplift-repositories" +#) def get_uplift_repositories(phab: PhabricatorClient) -> list: repos = phab.call_conduit( "diffusion.repository.search", diff --git a/src/lando/api/legacy/workers/landing_worker.py b/src/lando/api/legacy/workers/landing_worker.py index cbded17a..752afd37 100644 --- a/src/lando/api/legacy/workers/landing_worker.py +++ b/src/lando/api/legacy/workers/landing_worker.py @@ -103,17 +103,15 @@ def loop(self): ).first() if job is None: - db.session.commit() self.throttle(self.sleep_seconds) return with job_processing(self, job, db): job.status = LandingJobStatus.IN_PROGRESS job.attempts += 1 + job.save() # Make sure the status and attempt count are updated in the database - db.session.commit() - repo = repo_clone_subsystem.repos[job.repository_name] hgrepo = HgRepo( str(repo_clone_subsystem.repo_paths[job.repository_name]), @@ -271,8 +269,6 @@ def run_job( job.transition_status( LandingJobAction.DEFER, message=f"Tree {repo.tree} is closed - retrying later.", - commit=True, - db=db, ) return False @@ -288,7 +284,7 @@ def run_job( ) logger.exception(message) job.transition_status( - LandingJobAction.DEFER, message=message, commit=True, db=db + LandingJobAction.DEFER, message=message ) # Try again, this is a temporary failure. @@ -299,14 +295,12 @@ def run_job( job.transition_status( LandingJobAction.FAIL, message=message + f"\n{e}", - commit=True, - db=db, ) self.notify_user_of_landing_failure(job) return True # Run through the patches one by one and try to apply them. - for revision in job.revisions: + for revision in job.revisions.all(): patch_buf = StringIO(revision.patch_string) try: @@ -323,7 +317,7 @@ def run_job( ) logger.exception(message) job.transition_status( - LandingJobAction.FAIL, message=message, commit=True, db=db + LandingJobAction.FAIL, message=message ) self.notify_user_of_landing_failure(job) return True @@ -337,8 +331,6 @@ def run_job( job.transition_status( LandingJobAction.FAIL, message=message, - commit=True, - db=db, ) self.notify_user_of_landing_failure(job) return True @@ -351,8 +343,6 @@ def run_job( job.transition_status( LandingJobAction.FAIL, message=message, - commit=True, - db=db, ) self.notify_user_of_landing_failure(job) return True @@ -388,7 +378,7 @@ def run_job( logger.exception(message) job.transition_status( - LandingJobAction.FAIL, message=message, commit=True, db=db + LandingJobAction.FAIL, message=message ) self.notify_user_of_landing_failure(job) return False @@ -418,7 +408,7 @@ def run_job( ) logger.exception(message) job.transition_status( - LandingJobAction.DEFER, message=message, commit=True, db=db + LandingJobAction.DEFER, message=message ) return False # Try again, this is a temporary failure. except Exception as e: @@ -427,21 +417,19 @@ def run_job( job.transition_status( LandingJobAction.FAIL, message=message, - commit=True, - db=db, ) self.notify_user_of_landing_failure(job) return True # Do not try again, this is a permanent failure. job.transition_status( - LandingJobAction.LAND, commit_id=commit_id, commit=True, db=db + LandingJobAction.LAND, commit_id=commit_id ) mots_path = Path(hgrepo.path) / "mots.yaml" if mots_path.exists(): logger.info(f"{mots_path} found, setting reviewer data.") job.set_landed_reviewers(mots_path) - db.session.commit() + job.save() else: logger.info(f"{mots_path} not found, skipping setting reviewer data.") diff --git a/src/lando/api/tests/conftest.py b/src/lando/api/tests/conftest.py index 98fab788..191e7825 100644 --- a/src/lando/api/tests/conftest.py +++ b/src/lando/api/tests/conftest.py @@ -9,16 +9,15 @@ from pathlib import Path from types import SimpleNamespace -import flask.testing +from django.test import Client import pytest import redis import requests import requests_mock -from flask import current_app -from pytest_flask.plugin import JSONResponse +from django.http import JsonResponse as JSONResponse +from django.core.cache import cache -from lando.api.legacy.app import SUBSYSTEMS, construct_app, load_config -from lando.api.legacy.cache import cache +from lando import settings from lando.api.legacy.mocks.auth import TEST_JWKS, MockAuth0 from lando.api.legacy.phabricator import PhabricatorClient from lando.api.legacy.projects import ( @@ -28,10 +27,8 @@ SEC_PROJ_SLUG, ) from lando.api.legacy.repos import SCM_LEVEL_1, SCM_LEVEL_3, Repo -from lando.api.legacy.storage import db as _db -from lando.api.legacy.tasks import celery from lando.api.legacy.transplants import CODE_FREEZE_OFFSET, tokens_are_equal -from tests.mocks import PhabricatorDouble, TreeStatusDouble +from lando.api.tests.mocks import PhabricatorDouble, TreeStatusDouble PATCH_NORMAL_1 = r""" # HG changeset patch @@ -86,6 +83,10 @@ """.strip() +@pytest.fixture +def app(): + pass + @pytest.fixture def normal_patch(): """Return one of several "normal" patches.""" @@ -101,7 +102,7 @@ def _patch(number=0): return _patch -class JSONClient(flask.testing.FlaskClient): +class JSONClient(Client): """Custom Flask test client that sends JSON by default. HTTP methods have a 'json=...' keyword that will JSON-encode the @@ -227,40 +228,9 @@ def versionfile(tmpdir): return v -@pytest.fixture -def disable_migrations(monkeypatch): - """Disable the Alembic DB migrations system in the app during testing.""" - - class StubAlembic: - def __init__(self): - pass - - def init_app(self, app, db): - pass - - monkeypatch.setattr("landoapi.storage.migrate", StubAlembic()) - - -@pytest.fixture -def app(versionfile, docker_env_vars, disable_migrations, mocked_repo_config): - """Needed for pytest-flask.""" - config = load_config() - # We need the TESTING setting turned on to get tracebacks when testing API - # endpoints with the TestClient. - config["TESTING"] = True - config["CACHE_DISABLED"] = True - app = construct_app(config) - flask_app = app.app - flask_app.test_client_class = JSONClient - for system in SUBSYSTEMS: - system.init_app(flask_app) - - return flask_app - - @pytest.fixture def jwks(monkeypatch): - monkeypatch.setattr("landoapi.auth.get_jwks", lambda *args, **kwargs: TEST_JWKS) + monkeypatch.setattr("lando.api.legacy.auth.get_jwks", lambda *args, **kwargs: TEST_JWKS) @pytest.fixture @@ -270,7 +240,7 @@ def auth0_mock(jwks, monkeypatch): status_code=200, json=lambda: mock_auth0.userinfo ) monkeypatch.setattr( - "landoapi.auth.fetch_auth0_userinfo", lambda token: mock_userinfo_response + "lando.api.legacy.auth.fetch_auth0_userinfo", lambda token: mock_userinfo_response ) return mock_auth0 @@ -278,7 +248,7 @@ def auth0_mock(jwks, monkeypatch): @pytest.fixture def mock_repo_config(monkeypatch): def set_repo_config(config): - monkeypatch.setattr("landoapi.repos.REPO_CONFIG", config) + monkeypatch.setattr("lando.api.legacy.repos.REPO_CONFIG", config) return set_repo_config @@ -330,7 +300,7 @@ def set_value(val): mem["val"] = val monkeypatch.setattr( - "landoapi.transplants.tokens_are_equal", + "lando.api.legacy.transplants.tokens_are_equal", lambda t1, t2: mem["val"] if mem["set"] else tokens_are_equal(t1, t2), ) return set_value @@ -339,8 +309,8 @@ def set_value(val): @pytest.fixture def get_phab_client(app): def get_client(api_key=None): - api_key = api_key or current_app.config["PHABRICATOR_UNPRIVILEGED_API_KEY"] - return PhabricatorClient(current_app.config["PHABRICATOR_URL"], api_key) + api_key = api_key or settings.PHABRICATOR_UNPRIVILEGED_API_KEY + return PhabricatorClient(settings.PHABRICATOR_URL, api_key) return get_client @@ -362,21 +332,6 @@ def redis_cache(app): cache.init_app(app, config={"CACHE_TYPE": "null", "CACHE_NO_NULL_WARNING": True}) -@pytest.fixture -def celery_app(app): - """Configure our app's Celery instance for use with the celery_worker fixture.""" - # The test suite will fail if we don't override the default worker and - # default task set. - # Note: the test worker will fail if we don't specify a result_backend. The test - # harness uses the backend for a custom ping() task that it uses as a health check. - celery.conf.update(broker_url="memory://", result_backend="rpc") - # Workaround for https://github.com/celery/celery/issues/4032. If 'tasks.ping' is - # missing from the loaded task list then the test worker will fail with an - # AssertionError. - celery.loader.import_module("celery.contrib.testing.tasks") - return celery - - @pytest.fixture def treestatus_url(): """A string holding the Tree Status base URL.""" diff --git a/src/lando/api/tests/mocks.py b/src/lando/api/tests/mocks.py index b962af6c..8560ae0d 100644 --- a/src/lando/api/tests/mocks.py +++ b/src/lando/api/tests/mocks.py @@ -13,7 +13,7 @@ ReviewerStatus, ) from lando.api.legacy.treestatus import TreeStatus, TreeStatusError -from tests.canned_responses.phabricator.diffs import ( +from lando.api.tests.canned_responses.phabricator.diffs import ( CANNED_DEFAULT_DIFF_CHANGES, CANNED_RAW_DEFAULT_DIFF, ) diff --git a/src/lando/api/tests/test_decorators.py b/src/lando/api/tests/test_decorators.py index 4491102b..d14d2dfb 100644 --- a/src/lando/api/tests/test_decorators.py +++ b/src/lando/api/tests/test_decorators.py @@ -30,7 +30,7 @@ def test_require_phabricator_api_key(monkeypatch, app, optional, valid_key, stat if valid_key is not None: headers.append(("X-Phabricator-API-Key", "custom-key")) monkeypatch.setattr( - "landoapi.decorators.PhabricatorClient.verify_api_token", + "lando.api.legacy.decorators.PhabricatorClient.verify_api_token", lambda *args, **kwargs: valid_key, ) diff --git a/src/lando/api/tests/test_dockerflow.py b/src/lando/api/tests/test_dockerflow.py index d3242037..20528175 100644 --- a/src/lando/api/tests/test_dockerflow.py +++ b/src/lando/api/tests/test_dockerflow.py @@ -4,7 +4,7 @@ import json -from tests.utils import phab_url +from lando.api.tests.utils import phab_url def test_dockerflow_lb_endpoint_returns_200(client): diff --git a/src/lando/api/tests/test_health.py b/src/lando/api/tests/test_health.py deleted file mode 100644 index 29c2e554..00000000 --- a/src/lando/api/tests/test_health.py +++ /dev/null @@ -1,50 +0,0 @@ -# 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 http://mozilla.org/MPL/2.0/. - -from unittest.mock import Mock - -import redis - -from lando.api.legacy.auth import auth0_subsystem -from lando.api.legacy.cache import cache_subsystem -from lando.api.legacy.phabricator import PhabricatorAPIException, phabricator_subsystem - - -def test_phabricator_healthy(app, phabdouble): - assert phabricator_subsystem.healthy() is True - - -def test_phabricator_unhealthy(app, monkeypatch): - def raises(*args, **kwargs): - raise PhabricatorAPIException - - monkeypatch.setattr("landoapi.phabricator.PhabricatorClient.call_conduit", raises) - assert phabricator_subsystem.healthy() is not True - - -def test_cache_healthy(redis_cache): - assert cache_subsystem.healthy() is True - - -def test_cache_unhealthy_configuration(): - assert cache_subsystem.healthy() is not True - - -def test_cache_unhealthy_service(redis_cache, monkeypatch): - mock_cache = Mock(redis_cache) - mock_cache.cache._read_client.ping.side_effect = redis.TimeoutError - monkeypatch.setattr("landoapi.cache.cache", mock_cache) - monkeypatch.setattr("landoapi.cache.RedisCache", type(mock_cache.cache)) - - health = cache_subsystem.healthy() - assert health is not True - assert health.startswith("RedisError:") - - -def test_auth0_healthy(app, jwks): - assert auth0_subsystem.healthy() is True - - -def test_auth0_unhealthy(app): - assert auth0_subsystem.healthy() is not True diff --git a/src/lando/api/tests/test_landing_job.py b/src/lando/api/tests/test_landing_job.py index 90d59681..0d11e1a9 100644 --- a/src/lando/api/tests/test_landing_job.py +++ b/src/lando/api/tests/test_landing_job.py @@ -17,8 +17,7 @@ def _landing_job(status, requester_email="tuser@example.com"): requester_email=requester_email, repository_name="", ) - db.session.add(job) - db.session.commit() + job.save() return job return _landing_job @@ -91,7 +90,7 @@ def test_cancel_landing_job_fails_not_found(db, client, landing_job, auth0_mock) ) assert response.status_code == 404 - assert response.json["detail"] == ("A landing job with ID 1 was not found.") + assert response.json()["detail"] == ("A landing job with ID 1 was not found.") def test_cancel_landing_job_fails_bad_input(db, client, landing_job, auth0_mock): @@ -136,9 +135,7 @@ def test_landing_job_acquire_job_job_queue_query(db): ), ] for job in jobs: - db.session.add(job) - db.session.commit() - + job.save() # Queue order should match the order the jobs were created in. for qjob, job in zip(LandingJob.job_queue_query(repositories=[REPO_NAME]), jobs): assert qjob is job @@ -147,8 +144,9 @@ def test_landing_job_acquire_job_job_queue_query(db): # cancelled so that the queue changes. jobs[2].status = LandingJobStatus.IN_PROGRESS jobs[1].status = LandingJobStatus.CANCELLED - db.session.commit() + for job in jobs: + job.save() # The now IN_PROGRESS job should be first, and the cancelled job should # not appear in the queue. queue_items = LandingJob.job_queue_query( diff --git a/src/lando/api/tests/test_landings.py b/src/lando/api/tests/test_landings.py index b52a3362..2d389381 100644 --- a/src/lando/api/tests/test_landings.py +++ b/src/lando/api/tests/test_landings.py @@ -29,9 +29,8 @@ def _create_patch_revision(number, patch=normal_patch_0): revision = Revision() revision.revision_id = number revision.diff_id = number - revision.patch_bytes = patch.encode("utf-8") - db.session.add(revision) - db.session.commit() + revision.patch = patch + revision.save() return revision return _create_patch_revision @@ -221,6 +220,7 @@ def _create_patch_revision(number, patch=normal_patch_0): [(1, {"patch": LARGE_PATCH})], ], ) +@pytest.mark.django_db def test_integrated_execute_job( app, db, @@ -259,7 +259,7 @@ def test_integrated_execute_job( # Mock `phab_trigger_repo_update` so we can make sure that it was called. mock_trigger_update = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock_trigger_update, ) @@ -305,7 +305,7 @@ def test_integrated_execute_job_with_force_push( # We don't care about repo update in this test, however if we don't mock # this, the test will fail since there is no celery instance. monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock.MagicMock(), ) @@ -352,7 +352,7 @@ def test_integrated_execute_job_with_bookmark( # We don't care about repo update in this test, however if we don't mock # this, the test will fail since there is no celery instance. monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock.MagicMock(), ) @@ -440,7 +440,7 @@ def test_failed_landing_job_notification( # Mock `notify_user_of_landing_failure` so we can make sure that it was called. mock_notify = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.notify_user_of_landing_failure", mock_notify + "lando.api.legacy.workers.landing_worker.notify_user_of_landing_failure", mock_notify ) assert worker.run_job(job, repo, hgrepo, treestatus) @@ -547,7 +547,7 @@ def test_format_patch_success_unchanged( # Mock `phab_trigger_repo_update` so we can make sure that it was called. mock_trigger_update = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock_trigger_update, ) @@ -611,7 +611,7 @@ def test_format_single_success_changed( # Mock `phab_trigger_repo_update` so we can make sure that it was called. mock_trigger_update = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock_trigger_update, ) @@ -695,7 +695,7 @@ def test_format_stack_success_changed( # Mock `phab_trigger_repo_update` so we can make sure that it was called. mock_trigger_update = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock_trigger_update, ) @@ -776,7 +776,7 @@ def test_format_patch_fail( # Mock `notify_user_of_landing_failure` so we can make sure that it was called. mock_notify = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.notify_user_of_landing_failure", mock_notify + "lando.api.legacy.workers.landing_worker.notify_user_of_landing_failure", mock_notify ) assert not worker.run_job( @@ -834,14 +834,14 @@ def test_format_patch_no_landoini( # Mock `phab_trigger_repo_update` so we can make sure that it was called. mock_trigger_update = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.LandingWorker.phab_trigger_repo_update", + "lando.api.legacy.workers.landing_worker.LandingWorker.phab_trigger_repo_update", mock_trigger_update, ) # Mock `notify_user_of_landing_failure` so we can make sure that it was called. mock_notify = mock.MagicMock() monkeypatch.setattr( - "landoapi.workers.landing_worker.notify_user_of_landing_failure", mock_notify + "lando.api.legacy.workers.landing_worker.notify_user_of_landing_failure", mock_notify ) assert worker.run_job(job, repo, hgrepo, treestatus) @@ -877,6 +877,6 @@ def test_landing_job_revisions_sorting( assert job.revisions == revisions new_ordering = [revisions[2], revisions[0], revisions[1]] job.sort_revisions(new_ordering) - db.session.commit() + job.save() job = LandingJob.objects.get(id=job.id) assert job.revisions == new_ordering diff --git a/src/lando/api/tests/test_notifications.py b/src/lando/api/tests/test_notifications.py index 79881663..7ae1858c 100644 --- a/src/lando/api/tests/test_notifications.py +++ b/src/lando/api/tests/test_notifications.py @@ -5,7 +5,7 @@ import pytest -from lando.api.legacy.celery import FlaskCelery +from lando.api.legacy.celery import Celery from lando.api.legacy.email import make_failure_email from lando.main.models.landing_job import LandingJob from lando.main.models.revision import Revision @@ -55,7 +55,7 @@ def check_celery(app): @pytest.fixture def smtp(monkeypatch): client = FakeSMTP() - monkeypatch.setattr("landoapi.smtp.smtplib.SMTP", client) + monkeypatch.setattr("lando.api.legacy.smtp.smtplib.SMTP", client) return client @@ -143,7 +143,7 @@ def test_mail_suppress_send(app, smtp): def test_disabling_celery_keeps_tasks_from_executing(app): app.config["DISABLE_CELERY"] = True - celery = FlaskCelery() + celery = Celery() celery.init_app(app) assert celery.dispatch_disabled # Sanity check diff --git a/src/lando/api/tests/test_phabricator_client.py b/src/lando/api/tests/test_phabricator_client.py index 6214cde3..fe6537c8 100644 --- a/src/lando/api/tests/test_phabricator_client.py +++ b/src/lando/api/tests/test_phabricator_client.py @@ -10,7 +10,7 @@ import requests_mock from lando.api.legacy.phabricator import PhabricatorAPIException -from tests.utils import phab_url +from lando.api.tests.utils import phab_url pytestmark = pytest.mark.usefixtures("docker_env_vars") diff --git a/src/lando/api/tests/test_py3_codequality.py b/src/lando/api/tests/test_py3_codequality.py deleted file mode 100644 index 1ae3d97d..00000000 --- a/src/lando/api/tests/test_py3_codequality.py +++ /dev/null @@ -1,26 +0,0 @@ -# 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 http://mozilla.org/MPL/2.0/. -""" -Code Style Tests. -""" - -import subprocess - -from lando.api.legacy.cli import LINT_PATHS - - -def test_check_python_style(): - cmd = ("black", "--diff") - output = subprocess.check_output(cmd + LINT_PATHS) - assert not output, "The python code does not adhere to the project style." - - -def test_check_python_ruff(): - passed = [] - for lint_path in LINT_PATHS: - passed.append( - subprocess.call(("ruff", "check", lint_path, "--target-version", "py39")) - == 0 - ) - assert all(passed), "ruff did not run cleanly." diff --git a/src/lando/api/tests/test_transplants.py b/src/lando/api/tests/test_transplants.py index 8adbf8b8..e208bf8c 100644 --- a/src/lando/api/tests/test_transplants.py +++ b/src/lando/api/tests/test_transplants.py @@ -53,7 +53,8 @@ def _create_landing_job( revision = Revision(revision_id=revision_id) revision.diff_id = diff_id revisions.append(revision) - db.session.add_all(revisions) + for revision in revisions: + revision.save() job = add_job_with_revisions(revisions, **job_params) return job @@ -77,7 +78,7 @@ def _create_landing_job_with_no_linked_revisions( "status": status, } job = LandingJob(**job_params) - db.session.add(job) + job.save() revisions = [] for revision_id, diff_id in landing_path: revision = Revision.one_or_none(revision_id=revision_id) @@ -85,12 +86,13 @@ def _create_landing_job_with_no_linked_revisions( revision = Revision(revision_id=revision_id) revision.diff_id = diff_id revisions.append(revision) - db.session.add_all(revisions) + for revision in revisions: + revision.save() job.revision_to_diff_id = { str(revision.revision_id): revision.diff_id for revision in revisions } job.revision_order = [str(revision.revision_id) for r in revisions] - db.session.commit() + job.save() return job @@ -238,7 +240,7 @@ def test_dryrun_codefreeze_warn( "NEXT_MERGE_DATE": "tomorrow", }, ) - monkeypatch.setattr("landoapi.transplants.datetime", codefreeze_datetime()) + monkeypatch.setattr("lando.api.legacy.transplants.datetime", codefreeze_datetime()) mc_repo = Repo( tree="mozilla-conduit", url="https://hg.test/mozilla-conduit", @@ -248,7 +250,7 @@ def test_dryrun_codefreeze_warn( ) mc_mock = MagicMock() mc_mock.return_value = {"mozilla-central": mc_repo} - monkeypatch.setattr("landoapi.transplants.get_repos_for_env", mc_mock) + monkeypatch.setattr("lando.api.legacy.transplants.get_repos_for_env", mc_mock) d1 = phabdouble.diff() r1 = phabdouble.revision(diff=d1, repo=phabdouble.repo()) @@ -296,7 +298,7 @@ def test_dryrun_outside_codefreeze( "NEXT_MERGE_DATE": "five_weeks_from_today", }, ) - monkeypatch.setattr("landoapi.transplants.datetime", codefreeze_datetime()) + monkeypatch.setattr("lando.api.legacy.transplants.datetime", codefreeze_datetime()) mc_repo = Repo( tree="mozilla-conduit", url="https://hg.test/mozilla-conduit", @@ -306,7 +308,7 @@ def test_dryrun_outside_codefreeze( ) mc_mock = MagicMock() mc_mock.return_value = {"mozilla-central": mc_repo} - monkeypatch.setattr("landoapi.transplants.get_repos_for_env", mc_mock) + monkeypatch.setattr("lando.api.legacy.transplants.get_repos_for_env", mc_mock) d1 = phabdouble.diff() r1 = phabdouble.revision(diff=d1, repo=phabdouble.repo()) @@ -696,9 +698,6 @@ def test_integrated_transplant_simple_stack_saves_data_in_db( assert "id" in response.json job_id = response.json["id"] - # Ensure DB access isn't using uncommitted data. - db.session.close() - # Get LandingJob object by its id job = LandingJob.objects.get(pk=job_id) assert job.id == job_id @@ -741,12 +740,12 @@ def test_integrated_transplant_records_approvers_peers_and_owners( # Mock a few mots-related things needed by the landing worker. # First, mock path existance. mock_path = MagicMock() - monkeypatch.setattr("landoapi.workers.landing_worker.Path", mock_path) + monkeypatch.setattr("lando.api.legacy.workers.landing_worker.Path", mock_path) (mock_path(hgrepo.path) / "mots.yaml").exists.return_value = True # Then mock the directory/file config. mock_Directory = MagicMock() - monkeypatch.setattr("landoapi.models.landing_job.Directory", mock_Directory) + monkeypatch.setattr("lando.api.legacy.models.landing_job.Directory", mock_Directory) mock_Directory.return_value = MagicMock() mock_Directory().peers_and_owners = [101, 102] @@ -776,9 +775,6 @@ def test_integrated_transplant_records_approvers_peers_and_owners( assert "id" in response.json job_id = response.json["id"] - # Ensure DB access isn't using uncommitted data. - db.session.close() - # Get LandingJob object by its id job = LandingJob.objects.get(pk=job_id) assert job.id == job_id @@ -836,9 +832,6 @@ def test_integrated_transplant_updated_diff_id_reflected_in_landed_revisions( assert "id" in response.json job_1_id = response.json["id"] - # Ensure DB access isn't using uncommitted data. - db.session.close() - # Get LandingJob object by its id. job = LandingJob.objects.get(pk=job_1_id) assert job.id == job_1_id @@ -872,9 +865,6 @@ def test_integrated_transplant_updated_diff_id_reflected_in_landed_revisions( job_2_id = response.json["id"] - # Ensure DB access isn't using uncommitted data. - db.session.close() - # Get LandingJob objects by their ids. job_1 = LandingJob.objects.get(pk=job_1_id) job_2 = LandingJob.objects.get(pk=job_2_id) @@ -914,7 +904,7 @@ def test_integrated_transplant_with_flags( mock_format_commit_message = MagicMock() mock_format_commit_message.return_value = "Mock formatted commit message." monkeypatch.setattr( - "landoapi.api.transplants.format_commit_message", mock_format_commit_message + "lando.api.legacy.api.transplants.format_commit_message", mock_format_commit_message ) response = client.post( "/transplants", @@ -975,7 +965,7 @@ def test_integrated_transplant_legacy_repo_checkin_project_removed( mock_remove = MagicMock(admin_remove_phab_project) monkeypatch.setattr( - "landoapi.api.transplants.admin_remove_phab_project", mock_remove + "lando.api.legacy.api.transplants.admin_remove_phab_project", mock_remove ) response = client.post( @@ -1009,7 +999,7 @@ def test_integrated_transplant_repo_checkin_project_removed( mock_remove = MagicMock(admin_remove_phab_project) monkeypatch.setattr( - "landoapi.api.transplants.admin_remove_phab_project", mock_remove + "lando.api.legacy.api.transplants.admin_remove_phab_project", mock_remove ) response = client.post( diff --git a/src/lando/jinja2.py b/src/lando/jinja.py similarity index 100% rename from src/lando/jinja2.py rename to src/lando/jinja.py diff --git a/src/lando/main/admin.py b/src/lando/main/admin.py index f366d2db..1de98dbf 100644 --- a/src/lando/main/admin.py +++ b/src/lando/main/admin.py @@ -1,6 +1,8 @@ from django.contrib import admin -from lando.main.models import LandingJob, Repo, Revision, Worker +from lando.main.models.landing_job import LandingJob +from lando.main.models.revision import Revision +from lando.main.models.base import Repo, Worker admin.site.register(LandingJob, admin.ModelAdmin) admin.site.register(Revision, admin.ModelAdmin) diff --git a/src/lando/main/migrations/0001_initial.py b/src/lando/main/migrations/0001_initial.py index fbae22f8..d1b253a8 100644 --- a/src/lando/main/migrations/0001_initial.py +++ b/src/lando/main/migrations/0001_initial.py @@ -1,10 +1,11 @@ -# Generated by Django 5.0.1 on 2024-01-19 18:48 +# Generated by Django 5.0.3 on 2024-04-15 19:10 import django.db.models.deletion from django.db import migrations, models class Migration(migrations.Migration): + initial = True dependencies = [] diff --git a/src/lando/main/migrations/0002_configurationvariable.py b/src/lando/main/migrations/0002_configurationvariable.py new file mode 100644 index 00000000..247dfaf1 --- /dev/null +++ b/src/lando/main/migrations/0002_configurationvariable.py @@ -0,0 +1,48 @@ +# Generated by Django 5.0.3 on 2024-04-15 19:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0001_initial"), + ] + + operations = [ + migrations.CreateModel( + name="ConfigurationVariable", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("key", models.TextField(unique=True)), + ("raw_value", models.TextField(blank=True, default="")), + ( + "variable_type", + models.CharField( + blank=True, + choices=[ + ("BOOL", "Boolean"), + ("INT", "Integer"), + ("STR", "String"), + ], + default="STR", + max_length=4, + null=True, + ), + ), + ], + options={ + "abstract": False, + }, + ), + ] diff --git a/src/lando/main/models/__init__.py b/src/lando/main/models/__init__.py index c7022026..e4a8af86 100644 --- a/src/lando/main/models/__init__.py +++ b/src/lando/main/models/__init__.py @@ -1,3 +1 @@ -from lando.main.models.base import * -from lando.main.models.landing_job import * -from lando.main.models.revision import * +from lando.main.models.configuration import ConfigurationVariable diff --git a/src/lando/main/models/configuration.py b/src/lando/main/models/configuration.py index 0ddb6f7d..3adad1e0 100644 --- a/src/lando/main/models/configuration.py +++ b/src/lando/main/models/configuration.py @@ -8,7 +8,7 @@ from django.db import models from django.utils.translation import gettext_lazy -from lando.main.models import BaseModel +from lando.main.models.base import BaseModel logger = logging.getLogger(__name__) diff --git a/src/lando/main/models/landing_job.py b/src/lando/main/models/landing_job.py index eb74a05e..cc5588e9 100644 --- a/src/lando/main/models/landing_job.py +++ b/src/lando/main/models/landing_job.py @@ -17,7 +17,7 @@ from mots.config import FileConfig from mots.directory import Directory -from lando.main.models import BaseModel +from lando.main.models.base import BaseModel from lando.main.models.revision import Revision, RevisionLandingJob logger = logging.getLogger(__name__) @@ -204,7 +204,7 @@ def job_queue_query( q = cls.objects.filter(status__in=applicable_statuses) if repositories: - q = q.filter(target_repo__in=repositories) + q = q.filter(repository_name__in=repositories) if grace_seconds: now = datetime.datetime.now(datetime.timezone.utc) @@ -235,7 +235,7 @@ def add_revisions(self, revisions: list[Revision]): def sort_revisions(self, revisions: list[Revision]): """Sort the associated revisions based on provided list.""" - if len(revisions) != len(self.revisions): + if len(revisions) != len(self.revisions.all()): raise ValueError("List of revisions does not match associated revisions") # Update association table records with correct index values. diff --git a/src/lando/main/models/revision.py b/src/lando/main/models/revision.py index 5c533c83..0c9cd622 100644 --- a/src/lando/main/models/revision.py +++ b/src/lando/main/models/revision.py @@ -13,7 +13,7 @@ from django.db import models from django.utils.translation import gettext_lazy -from lando.main.models import BaseModel +from lando.main.models.base import BaseModel from lando.utils import build_patch_for_revision logger = logging.getLogger(__name__) diff --git a/src/lando/main/support.py b/src/lando/main/support.py index 414a1968..61e1b72c 100644 --- a/src/lando/main/support.py +++ b/src/lando/main/support.py @@ -2,7 +2,7 @@ class ProblemException(Exception): - def __init__(self, *, status=500, title=None, detail=None, type=None, instance=None, headers=None, ext=None): + def __init__(self, status=500, title=None, detail=None, type=None, instance=None, headers=None, ext=None): # TODO: this should be reimplemented as either middleware or HttpResponse return values. super().__init__(self) @@ -32,4 +32,4 @@ def get_response(self, _problem): class ConnexionResponse(HttpResponse): - pass \ No newline at end of file + pass diff --git a/src/lando/settings.py b/src/lando/settings.py index 7b3f5ef8..da46926c 100644 --- a/src/lando/settings.py +++ b/src/lando/settings.py @@ -59,7 +59,7 @@ "DIRS": [], "APP_DIRS": True, "OPTIONS": { - "environment": "lando.jinja2.environment" + "environment": "lando.jinja.environment" }, }, { @@ -162,3 +162,4 @@ LINT_PATHS = tuple(f"{BASE_DIR}/{path}" for path in ("main", "utils", "api")) GITHUB_ACCESS_TOKEN = os.getenv("LANDO_GITHUB_ACCESS_TOKEN") +PHABRICATOR_URL = os.getenv("PHABRICATOR_URL", "")