Skip to content

Commit

Permalink
testing: fix up various aspects that are breaking tests (bug 1887042)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
zzzeid committed Apr 18, 2024
1 parent eb6f15c commit 9874df0
Show file tree
Hide file tree
Showing 31 changed files with 184 additions and 344 deletions.
18 changes: 17 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ 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
Expand All @@ -22,7 +35,10 @@ jobs:
python -m pip install -r requirements.txt
python -m pip install -e .
- name: Test
env:
DEFAULT_DB_HOST: 127.0.0.1
run: |
source env/bin/activate
lando migrate
lando test
pytest
pytest src/lando/api
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ build-backend = "setuptools.build_meta"

[tool.setuptools.packages.find]
where = ["src"]

[tool.pytest.ini_options]
DJANGO_SETTINGS_MODULE = "lando.settings"
9 changes: 9 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
black
jinja2
kombu
pytest
pytest-django
python-hglib
python-jose
redis
requests-mock
ruff
uwsgi
celery
datadog
rs-parsepatch
networkx
1 change: 0 additions & 1 deletion src/lando/api/legacy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
113 changes: 12 additions & 101 deletions src/lando/api/legacy/celery.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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()
15 changes: 6 additions & 9 deletions src/lando/api/legacy/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()
Expand All @@ -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."""

Expand Down Expand Up @@ -83,21 +80,21 @@ 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"
)


@cli.command(name="run-post-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"
)


Expand Down
2 changes: 1 addition & 1 deletion src/lando/api/legacy/mocks/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))])
7 changes: 4 additions & 3 deletions src/lando/api/legacy/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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


Expand Down
9 changes: 4 additions & 5 deletions src/lando/api/legacy/uplift.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit 9874df0

Please sign in to comment.