Skip to content

Commit 07f81da

Browse files
committed
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
1 parent eb6f15c commit 07f81da

21 files changed

+57
-251
lines changed

.github/workflows/build.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ jobs:
2525
run: |
2626
source env/bin/activate
2727
lando test
28-
pytest
28+
pytest /code/src/lando/api

pyproject.toml

+3
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,6 @@ build-backend = "setuptools.build_meta"
3232

3333
[tool.setuptools.packages.find]
3434
where = ["src"]
35+
36+
[tool.pytest.ini_options]
37+
DJANGO_SETTINGS_MODULE = "lando.settings"

requirements.txt

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
black
22
jinja2
3+
kombu
34
pytest
45
pytest-django
6+
python-hglib
7+
python-jose
8+
redis
9+
requests-mock
510
ruff
611
uwsgi
12+
celery
13+
datadog

src/lando/api/legacy/celery.py

+12-101
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
# This Source Code Form is subject to the terms of the Mozilla Public
22
# License, v. 2.0. If a copy of the MPL was not distributed with this
33
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
4+
import os
45
import logging
56

6-
import flask
77
from celery import Celery
88
from celery.signals import (
99
after_task_publish,
@@ -15,87 +15,24 @@
1515
)
1616
from datadog import statsd
1717

18-
from lando.api.legacy.systems import Subsystem
19-
2018
logger = logging.getLogger(__name__)
2119

2220

23-
class FlaskCelery(Celery):
24-
"""Celery which executes task in a flask app context."""
25-
26-
def __init__(self, *args, **kwargs):
27-
# Avoid passing the flask app to base Celery.
28-
flask_app = kwargs.pop("app", None)
29-
30-
super().__init__(*args, **kwargs)
31-
32-
# Important to run this after __init__ since task_cls
33-
# argument to base Celery can change what we're basing on.
34-
self._flask_override_task_class()
35-
36-
if flask_app is not None:
37-
self.init_app(flask_app)
38-
39-
@property
40-
def dispatch_disabled(self):
41-
"""Will the Celery job system dispatch tasks to the workers?"""
42-
return bool(self.app.config.get("DISABLE_CELERY"))
43-
44-
def init_app(self, app, config=None):
45-
"""Initialize with a flask app."""
46-
self.app = app
47-
48-
config = config or {}
49-
self.conf.update(main=app.import_name, **config)
50-
51-
if self.dispatch_disabled:
52-
logger.warning(
53-
"DISABLE_CELERY application configuration variable set, the Celery job "
54-
"system has been disabled! Any features that depend on the job system "
55-
"will not function."
56-
)
57-
58-
def _flask_override_task_class(self):
59-
"""Change Task class to one which executes in a flask context."""
60-
# Define a Task subclass that saves a reference to self in the Task object so
61-
# the task object can find self.app (the Flask application object) even if
62-
# self.app hasn't been set yet.
63-
#
64-
# We need to delay all of the task's calls to self.app using a custom Task class
65-
# because the reference to self.app may not be valid at the time the Celery
66-
# application object creates it set of Task objects. The programmer may
67-
# set self.app via the self.init_app() method at any time in the future.
68-
#
69-
# self.app is expected to be valid and usable by Task objects after the web
70-
# application is fully initialized and ready to serve requests.
71-
BaseTask = self.Task
72-
celery_self = self
73-
74-
class FlaskTask(BaseTask):
75-
"""A Celery Task subclass that has a reference to a Flask app."""
21+
# Set the default Django settings module for the 'celery' program.
22+
os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'lando.settings')
7623

77-
def __call__(self, *args, **kwargs):
78-
# Override immediate calling of tasks, such as mytask(). This call
79-
# method is used by the Celery worker process.
80-
if flask.has_app_context():
81-
return super().__call__(*args, **kwargs)
24+
app = Celery('lando')
8225

83-
with celery_self.app.app_context():
84-
return super().__call__(*args, **kwargs)
26+
# Using a string here means the worker doesn't have to serialize
27+
# the configuration object to child processes.
28+
# - namespace='CELERY' means all celery-related configuration keys
29+
# should have a `CELERY_` prefix.
30+
app.config_from_object('django.conf:settings', namespace='CELERY')
8531

86-
def apply_async(self, *args, **kwargs):
87-
# Override delayed calling of tasks, such as mytask.apply_async().
88-
# This call method is used by the Celery app when it wants to
89-
# schedule a job for eventual execution on a worker.
90-
if celery_self.dispatch_disabled:
91-
return None
92-
else:
93-
return super().apply_async(*args, **kwargs)
32+
# Load task modules from all registered Django apps.
33+
app.autodiscover_tasks()
9434

95-
self.Task = FlaskTask
96-
97-
98-
celery = FlaskCelery()
35+
celery = app
9936

10037

10138
@after_task_publish.connect
@@ -129,29 +66,3 @@ def count_task_retried(**kwargs):
12966
def setup_celery_logging(**kwargs):
13067
# Prevent celery from overriding our logging configuration.
13168
pass
132-
133-
134-
class CelerySubsystem(Subsystem):
135-
name = "celery"
136-
137-
def init_app(self, app):
138-
super().init_app(app)
139-
140-
# Import tasks to discover celery tasks.
141-
import landoapi.tasks # noqa
142-
143-
celery.init_app(
144-
self.flask_app,
145-
config={"broker_url": settings.CELERY_BROKER_URL},
146-
)
147-
celery.log.setup()
148-
149-
def ready(self):
150-
if settings.DISABLE_CELERY:
151-
return True
152-
153-
# TODO: Check connection to CELERY_BROKER_URL
154-
return True
155-
156-
157-
celery_subsystem = CelerySubsystem()

src/lando/api/legacy/cli.py

+6-9
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@
88
from typing import Optional
99

1010
import click
11-
import connexion
12-
from flask.cli import FlaskGroup
1311

1412
from lando.main.models.configuration import (
1513
ConfigurationKey,
1614
ConfigurationVariable,
17-
VariableType,
15+
VariableTypeChoices,
1816
)
1917
from lando.api.legacy.systems import Subsystem
2018

@@ -35,7 +33,7 @@ def get_subsystems(exclude: Optional[list[Subsystem]] = None) -> list[Subsystem]
3533
return [s for s in SUBSYSTEMS if s not in exclusions]
3634

3735

38-
def create_lando_api_app() -> connexion.App:
36+
def create_lando_api_app():
3937
from lando.api.legacy.app import construct_app, load_config
4038

4139
config = load_config()
@@ -46,7 +44,6 @@ def create_lando_api_app() -> connexion.App:
4644
return app.app
4745

4846

49-
@click.group(cls=FlaskGroup, create_app=create_lando_api_app)
5047
def cli():
5148
"""Lando API cli."""
5249

@@ -83,21 +80,21 @@ def landing_worker():
8380
def run_pre_deploy_sequence():
8481
"""Runs the sequence of commands required before a deployment."""
8582
ConfigurationVariable.set(
86-
ConfigurationKey.API_IN_MAINTENANCE, VariableType.BOOL, "1"
83+
ConfigurationKey.API_IN_MAINTENANCE, VariableTypeChoices.BOOL, "1"
8784
)
8885
ConfigurationVariable.set(
89-
ConfigurationKey.LANDING_WORKER_PAUSED, VariableType.BOOL, "1"
86+
ConfigurationKey.LANDING_WORKER_PAUSED, VariableTypeChoices.BOOL, "1"
9087
)
9188

9289

9390
@cli.command(name="run-post-deploy-sequence")
9491
def run_post_deploy_sequence():
9592
"""Runs the sequence of commands required after a deployment."""
9693
ConfigurationVariable.set(
97-
ConfigurationKey.API_IN_MAINTENANCE, VariableType.BOOL, "0"
94+
ConfigurationKey.API_IN_MAINTENANCE, VariableTypeChoices.BOOL, "0"
9895
)
9996
ConfigurationVariable.set(
100-
ConfigurationKey.LANDING_WORKER_PAUSED, VariableType.BOOL, "0"
97+
ConfigurationKey.LANDING_WORKER_PAUSED, VariableTypeChoices.BOOL, "0"
10198
)
10299

103100

src/lando/api/legacy/projects.py

+4-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
import logging
55
from typing import Optional
66

7-
from lando.api.legacy.cache import DEFAULT_CACHE_KEY_TIMEOUT_SECONDS, cache
7+
from django.core.cache import cache
8+
89
from lando.api.legacy.phabricator import PhabricatorClient, result_list_to_phid_dict
910

1011
logger = logging.getLogger(__name__)
@@ -57,7 +58,7 @@ def project_search(
5758
"project.search", constraints={"phids": project_phids}
5859
)
5960
result = result_list_to_phid_dict(phabricator.expect(projects, "data"))
60-
cache.set(cache_key, result, timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS)
61+
cache.set(cache_key, result)
6162
return result
6263

6364

@@ -92,7 +93,7 @@ def get_project_phid(
9293
)
9394

9495
value = phabricator.expect(project, "phid") if project else None
95-
cache.set(key, value, timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS)
96+
cache.set(key, value)
9697
return value
9798

9899

src/lando/api/legacy/uplift.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@
1818
Version,
1919
)
2020

21-
from lando.api import bmo
22-
from lando.api.legacy.cache import DEFAULT_CACHE_KEY_TIMEOUT_SECONDS, cache
21+
from lando.api.legacy import bmo
2322
from lando.api.legacy.phabricator import PhabricatorClient
2423
from lando.api.legacy.phabricator_patch import patch_to_changes
2524
from lando.api.legacy.repos import (
@@ -68,9 +67,9 @@ def get_uplift_request_form(revision: dict) -> Optional[str]:
6867
return bug
6968

7069

71-
@cache.cached(
72-
key_prefix="uplift-repositories", timeout=DEFAULT_CACHE_KEY_TIMEOUT_SECONDS
73-
)
70+
#@cache.cached(
71+
# key_prefix="uplift-repositories"
72+
#)
7473
def get_uplift_repositories(phab: PhabricatorClient) -> list:
7574
repos = phab.call_conduit(
7675
"diffusion.repository.search",

src/lando/api/tests/conftest.py

+8-43
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@
99
from pathlib import Path
1010
from types import SimpleNamespace
1111

12-
import flask.testing
12+
from django.test import Client
1313
import pytest
1414
import redis
1515
import requests
1616
import requests_mock
17-
from flask import current_app
18-
from pytest_flask.plugin import JSONResponse
17+
from django.http import JsonResponse as JSONResponse
18+
from django.core.cache import cache
1919

20-
from lando.api.legacy.app import SUBSYSTEMS, construct_app, load_config
21-
from lando.api.legacy.cache import cache
20+
from lando import settings
2221
from lando.api.legacy.mocks.auth import TEST_JWKS, MockAuth0
2322
from lando.api.legacy.phabricator import PhabricatorClient
2423
from lando.api.legacy.projects import (
@@ -28,10 +27,8 @@
2827
SEC_PROJ_SLUG,
2928
)
3029
from lando.api.legacy.repos import SCM_LEVEL_1, SCM_LEVEL_3, Repo
31-
from lando.api.legacy.storage import db as _db
32-
from lando.api.legacy.tasks import celery
3330
from lando.api.legacy.transplants import CODE_FREEZE_OFFSET, tokens_are_equal
34-
from tests.mocks import PhabricatorDouble, TreeStatusDouble
31+
from lando.api.tests.mocks import PhabricatorDouble, TreeStatusDouble
3532

3633
PATCH_NORMAL_1 = r"""
3734
# HG changeset patch
@@ -101,7 +98,7 @@ def _patch(number=0):
10198
return _patch
10299

103100

104-
class JSONClient(flask.testing.FlaskClient):
101+
class JSONClient(Client):
105102
"""Custom Flask test client that sends JSON by default.
106103
107104
HTTP methods have a 'json=...' keyword that will JSON-encode the
@@ -241,23 +238,6 @@ def init_app(self, app, db):
241238
monkeypatch.setattr("landoapi.storage.migrate", StubAlembic())
242239

243240

244-
@pytest.fixture
245-
def app(versionfile, docker_env_vars, disable_migrations, mocked_repo_config):
246-
"""Needed for pytest-flask."""
247-
config = load_config()
248-
# We need the TESTING setting turned on to get tracebacks when testing API
249-
# endpoints with the TestClient.
250-
config["TESTING"] = True
251-
config["CACHE_DISABLED"] = True
252-
app = construct_app(config)
253-
flask_app = app.app
254-
flask_app.test_client_class = JSONClient
255-
for system in SUBSYSTEMS:
256-
system.init_app(flask_app)
257-
258-
return flask_app
259-
260-
261241
@pytest.fixture
262242
def jwks(monkeypatch):
263243
monkeypatch.setattr("landoapi.auth.get_jwks", lambda *args, **kwargs: TEST_JWKS)
@@ -339,8 +319,8 @@ def set_value(val):
339319
@pytest.fixture
340320
def get_phab_client(app):
341321
def get_client(api_key=None):
342-
api_key = api_key or current_app.config["PHABRICATOR_UNPRIVILEGED_API_KEY"]
343-
return PhabricatorClient(current_app.config["PHABRICATOR_URL"], api_key)
322+
api_key = api_key or settings.PHABRICATOR_UNPRIVILEGED_API_KEY
323+
return PhabricatorClient(settings.PHABRICATOR_URL, api_key)
344324

345325
return get_client
346326

@@ -362,21 +342,6 @@ def redis_cache(app):
362342
cache.init_app(app, config={"CACHE_TYPE": "null", "CACHE_NO_NULL_WARNING": True})
363343

364344

365-
@pytest.fixture
366-
def celery_app(app):
367-
"""Configure our app's Celery instance for use with the celery_worker fixture."""
368-
# The test suite will fail if we don't override the default worker and
369-
# default task set.
370-
# Note: the test worker will fail if we don't specify a result_backend. The test
371-
# harness uses the backend for a custom ping() task that it uses as a health check.
372-
celery.conf.update(broker_url="memory://", result_backend="rpc")
373-
# Workaround for https://github.com/celery/celery/issues/4032. If 'tasks.ping' is
374-
# missing from the loaded task list then the test worker will fail with an
375-
# AssertionError.
376-
celery.loader.import_module("celery.contrib.testing.tasks")
377-
return celery
378-
379-
380345
@pytest.fixture
381346
def treestatus_url():
382347
"""A string holding the Tree Status base URL."""

src/lando/api/tests/mocks.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
ReviewerStatus,
1414
)
1515
from lando.api.legacy.treestatus import TreeStatus, TreeStatusError
16-
from tests.canned_responses.phabricator.diffs import (
16+
from lando.api.tests.canned_responses.phabricator.diffs import (
1717
CANNED_DEFAULT_DIFF_CHANGES,
1818
CANNED_RAW_DEFAULT_DIFF,
1919
)

src/lando/api/tests/test_dockerflow.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import json
66

7-
from tests.utils import phab_url
7+
from lando.api.tests.utils import phab_url
88

99

1010
def test_dockerflow_lb_endpoint_returns_200(client):

0 commit comments

Comments
 (0)