Skip to content

Commit 0f6f024

Browse files
committed
testing: fix up various aspects that are breaking tests (bug 1887042)
WIP DO NOT MERGE Skip following modules: - test_auth - test_dockerflow - test_notifications - test_diff_warnings - test_landing_job (partial) - fix ups to get tests to run - remove flask celery functionality - add boilerplate django celery impl - add db to github workflow
1 parent 222b7b1 commit 0f6f024

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+343
-394
lines changed

.github/workflows/build.yml

+17-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ on:
88
jobs:
99
run-tests:
1010
runs-on: ubuntu-latest
11+
services:
12+
postgres:
13+
image: postgres:latest
14+
env:
15+
POSTGRES_USER: postgres
16+
POSTGRES_PASSWORD: postgres
17+
ports:
18+
- 5432:5432
19+
options:
20+
--health-cmd pg_isready
21+
--health-interval 10s
22+
--health-timeout 5s
23+
--health-retries 5
1124
steps:
1225
- uses: actions/checkout@v4
1326
- name: Set up Python
@@ -22,7 +35,10 @@ jobs:
2235
python -m pip install -r requirements.txt
2336
python -m pip install -e .
2437
- name: Test
38+
env:
39+
DEFAULT_DB_HOST: 127.0.0.1
2540
run: |
2641
source env/bin/activate
42+
lando migrate
2743
lando test
28-
pytest
44+
pytest src/lando/api

Dockerfile

+11
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,20 @@ EXPOSE 80
44
ENV PYTHONUNBUFFERED=1
55
ENV PYTHONDONTWRITEBYTECODE 1
66
RUN adduser --system --no-create-home app
7+
78
RUN mkdir /code
89
COPY ./ /code
910
RUN pip install --upgrade pip
11+
12+
13+
# Install the Rust toolchain. Some packages do not have pre-built wheels (e.g.
14+
# rs-parsepatch) and require this in order to compile.
15+
RUN curl https://sh.rustup.rs -sSf | sh -s -- -y
16+
17+
# Include ~/.cargo/bin in PATH.
18+
# See: rust-lang.org/tools/install (Configuring the PATH environment variable).
19+
ENV PATH="/root/.cargo/bin:${PATH}"
20+
1021
RUN pip install -r /code/requirements.txt
1122
RUN pip install -e /code
1223
USER app

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

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

src/lando/api/legacy/api/transplants.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from lando.main.support import ProblemException, problem, g
1111
from lando import settings
1212

13-
from lando.api import auth
13+
from lando.api.legacy import auth
1414
from lando.api.legacy.commit_message import format_commit_message
1515
from lando.api.legacy.decorators import require_phabricator_api_key
1616
from lando.main.models.landing_job import (

src/lando/api/legacy/app.py

-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import connexion
1010
from connexion.resolver import RestyResolver
1111

12-
import landoapi.models # noqa, makes sure alembic knows about the models.
1312
from lando.api.legacy.auth import auth0_subsystem
1413
from lando.api.legacy.cache import cache_subsystem
1514
from lando.api.legacy.celery import celery_subsystem

src/lando/api/legacy/auth.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232

3333

3434
def get_auth_token() -> str:
35-
auth = request.headers.get("Authorization")
35+
auth = request["headers"].get("Authorization")
3636
if auth is None:
3737
raise ProblemException(
3838
401,

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/decorators.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def __init__(self, optional: bool = False, provide_client: bool = True):
3737
def __call__(self, f: Callable) -> Callable:
3838
@functools.wraps(f)
3939
def wrapped(*args, **kwargs):
40-
api_key = request.headers.get("X-Phabricator-API-Key")
40+
api_key = request["headers"].get("X-Phabricator-API-Key")
4141

4242
if api_key is None and not self.optional:
4343
return problem(

src/lando/api/legacy/hg.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class HgRepo:
165165
"extensions.purge": "",
166166
"extensions.strip": "",
167167
"extensions.rebase": "",
168-
"extensions.set_landing_system": "/app/hgext/set_landing_system.py",
168+
"extensions.set_landing_system": "/code/src/lando/api/legacy/hgext/set_landing_system.py",
169169
}
170170

171171
def __init__(self, path, config=None):

src/lando/api/legacy/mocks/auth.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,4 @@ def access_token(self):
9090

9191
@property
9292
def mock_headers(self):
93-
return [("Authorization", "Bearer {}".format(self.access_token))]
93+
return dict([("Authorization", "Bearer {}".format(self.access_token))])

src/lando/api/legacy/projects.py

+6-5
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__)
@@ -50,14 +51,14 @@ def project_search(
5051
project_phids.sort()
5152
cache_key = ",".join(project_phids)
5253

53-
if cache.has(cache_key):
54+
if cache.has_key(cache_key):
5455
return cache.get(cache_key)
5556

5657
projects = phabricator.call_conduit(
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

@@ -80,7 +81,7 @@ def get_project_phid(
8081
A string with the project's PHID or None if the project isn't found.
8182
"""
8283
key = f"PROJECT_{project_slug}"
83-
if cache.has(key):
84+
if cache.has_key(key):
8485
return cache.get(key)
8586

8687
project = phabricator.single(
@@ -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/transplants.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,8 @@ def warning_previously_landed(*, revision, diff, **kwargs):
216216

217217
job = (
218218
LandingJob.revisions_query([revision_id])
219-
.filter_by(status=LandingJobStatus.LANDED)
220-
.order_by(LandingJob.updated_at.desc())
219+
.filter(status=LandingJobStatus.LANDED)
220+
.order_by("-updated_at")
221221
.first()
222222
)
223223

@@ -233,7 +233,7 @@ def warning_previously_landed(*, revision, diff, **kwargs):
233233
revision_to_diff_id.update(legacy_data)
234234
landed_diff_id = revision_to_diff_id[revision_id]
235235
same = diff_id == landed_diff_id
236-
only_revision = len(job.revisions) == 1
236+
only_revision = job.revisions.count() == 1
237237

238238
return (
239239
"Already landed with {is_same_string} diff ({landed_diff_id}), "

0 commit comments

Comments
 (0)