From f19953f3e4254b23f584b20b7accb0329c4ca6e7 Mon Sep 17 00:00:00 2001 From: Cody Fincher <204685+cofin@users.noreply.github.com> Date: Wed, 11 Jan 2023 14:56:03 +0000 Subject: [PATCH 1/4] feat(worker): Allow multiple queues per worker configuration Updated namespace configuration to allow for multiple queus in an app. --- src/starlite_saqlalchemy/worker.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/starlite_saqlalchemy/worker.py b/src/starlite_saqlalchemy/worker.py index 73608d66..de8f0305 100644 --- a/src/starlite_saqlalchemy/worker.py +++ b/src/starlite_saqlalchemy/worker.py @@ -47,11 +47,22 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: *args: Passed through to `saq.Queue.__init__()` **kwargs: Passed through to `saq.Queue.__init__()` """ - kwargs.setdefault("name", settings.app.slug) + kwargs.setdefault("name", "background-worker") kwargs.setdefault("dump", encoder.encode) kwargs.setdefault("load", msgspec.json.decode) super().__init__(*args, **kwargs) + def namespace(self, key: str) -> str: + """Namespace for the Queue. + + Args: + key (str): The unique key to use for the namespace. + + Returns: + str: The worker namespace + """ + return f"{settings.app.slug}:{self.name}:{key}" + class Worker(saq.Worker): """Modify behavior of saq worker for orchestration by Starlite.""" From ab8e07dbd56c7cc2042c9ec2079a51ac3974986d Mon Sep 17 00:00:00 2001 From: gazorby Date: Mon, 9 Jan 2023 16:38:52 +0100 Subject: [PATCH 2/4] =?UTF-8?q?=E2=9C=A8=20feat(scripts):=20auto=20reload?= =?UTF-8?q?=20when=20ENVIRONMENT=3D=3Dlocal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/starlite_saqlalchemy/scripts.py | 43 +++++++++++++++++++++------- src/starlite_saqlalchemy/settings.py | 2 +- src/starlite_saqlalchemy/testing.py | 20 +++++++++++++ tests/unit/test_scripts.py | 25 ++++++++++++++++ 4 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_scripts.py diff --git a/src/starlite_saqlalchemy/scripts.py b/src/starlite_saqlalchemy/scripts.py index 6c741657..690837cc 100644 --- a/src/starlite_saqlalchemy/scripts.py +++ b/src/starlite_saqlalchemy/scripts.py @@ -3,6 +3,7 @@ # pylint: disable=broad-except import argparse import asyncio +from typing import TYPE_CHECKING import uvicorn import uvloop @@ -11,6 +12,11 @@ from starlite_saqlalchemy import redis, settings from starlite_saqlalchemy.db import engine +if TYPE_CHECKING: + from collections.abc import Coroutine + from typing import Any + + asyncio.set_event_loop_policy(uvloop.EventLoopPolicy()) @@ -41,6 +47,31 @@ async def _redis_ready() -> None: break +def _get_uvicorn_config() -> uvicorn.Config: + reload = ( + settings.server.RELOAD + if settings.server.RELOAD is not None + else settings.app.ENVIRONMENT == "local" + ) + reload_dirs = settings.server.RELOAD_DIRS if reload else None + + return uvicorn.Config( + app=settings.server.APP_LOC, + factory=settings.server.APP_LOC_IS_FACTORY, + host=settings.server.HOST, + loop="none", + port=settings.server.PORT, + reload=reload, + reload_dirs=reload_dirs, + timeout_keep_alive=settings.server.KEEPALIVE, + ) + + +async def _run_server(config: uvicorn.Config) -> "Coroutine[Any, Any, None]": + server = uvicorn.Server(config) + await server.serve() + + def run_app() -> None: """Run the application.""" parser = argparse.ArgumentParser(description="Run the application") @@ -49,18 +80,10 @@ def run_app() -> None: "--no-cache", action="store_const", const=False, default=True, dest="check_cache" ) args = parser.parse_args() + with asyncio.Runner() as runner: if args.check_db: runner.run(_db_ready()) if args.check_cache: runner.run(_redis_ready()) - uvicorn.run( - app=settings.server.APP_LOC, - factory=settings.server.APP_LOC_IS_FACTORY, - host=settings.server.HOST, - loop="none", - port=settings.server.PORT, - reload=settings.server.RELOAD, - reload_dirs=settings.server.RELOAD_DIRS, - timeout_keep_alive=settings.server.KEEPALIVE, - ) + runner.run(_run_server(_get_uvicorn_config())) diff --git a/src/starlite_saqlalchemy/settings.py b/src/starlite_saqlalchemy/settings.py index 136630e0..4c8e75df 100644 --- a/src/starlite_saqlalchemy/settings.py +++ b/src/starlite_saqlalchemy/settings.py @@ -235,7 +235,7 @@ class Config: """Seconds to hold connections open (65 is > AWS lb idle timeout).""" PORT: int = 8000 """Server port.""" - RELOAD: bool = False + RELOAD: bool | None = None """Turn on hot reloading.""" RELOAD_DIRS: list[str] = ["src/"] """Directories to watch for reloading.""" diff --git a/src/starlite_saqlalchemy/testing.py b/src/starlite_saqlalchemy/testing.py index a2a798ff..6162275c 100644 --- a/src/starlite_saqlalchemy/testing.py +++ b/src/starlite_saqlalchemy/testing.py @@ -5,6 +5,7 @@ from __future__ import annotations import random +from contextlib import contextmanager from datetime import datetime from typing import TYPE_CHECKING, Any, Generic, TypeVar from uuid import uuid4 @@ -18,6 +19,7 @@ if TYPE_CHECKING: from collections.abc import Callable, Hashable, Iterable, MutableMapping, Sequence + from pydantic import BaseSettings from pytest import MonkeyPatch from starlite.testing import TestClient @@ -28,6 +30,24 @@ MockRepoT = TypeVar("MockRepoT", bound="GenericMockRepository") +@contextmanager +def modify_settings(*update: tuple[BaseSettings, dict[str, Any]]): + old_settings: list[tuple[BaseSettings, dict[str, Any]]] = [] + try: + for model, new_values in update: + old_values = {} + for field, value in model.dict().items(): + if field in new_values: + old_values[field] = value + setattr(model, field, new_values[field]) + old_settings.append((model, old_values)) + yield + finally: + for model, old_values in old_settings: + for field, old_val in old_values.items(): + setattr(model, field, old_val) + + class GenericMockRepository(AbstractRepository[ModelT], Generic[ModelT]): """A repository implementation for tests. diff --git a/tests/unit/test_scripts.py b/tests/unit/test_scripts.py new file mode 100644 index 00000000..961a45da --- /dev/null +++ b/tests/unit/test_scripts.py @@ -0,0 +1,25 @@ +from pathlib import Path + +import pytest + +from starlite_saqlalchemy import settings +from starlite_saqlalchemy.scripts import _get_uvicorn_config +from starlite_saqlalchemy.testing import modify_settings + + +@pytest.mark.parametrize(("reload", "expected"), [(None, True), (True, True), (False, False)]) +def test_uvicorn_config_auto_reload_local(reload, expected): + with modify_settings( + (settings.app, {"ENVIRONMENT": "local"}), (settings.server, {"RELOAD": reload}) + ): + config = _get_uvicorn_config() + assert config.reload is expected + + +@pytest.mark.parametrize( + ("reload", "expected"), [(None, []), (True, settings.server.RELOAD_DIRS), (False, [])] +) +def test_uvicorn_config_reload_dirs(reload, expected): + with modify_settings((settings.server, {"RELOAD": reload})): + config = _get_uvicorn_config() + assert config.reload_dirs == [Path(path).absolute() for path in expected] From 8f6c80b5cde8e47785dc42bd01b379875a79c648 Mon Sep 17 00:00:00 2001 From: gazorby Date: Mon, 9 Jan 2023 17:08:38 +0100 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=90=9B=20fix:=20typing=20and=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/starlite_saqlalchemy/scripts.py | 9 ++------- src/starlite_saqlalchemy/testing.py | 22 +++++++++++++++++++--- tests/unit/test_scripts.py | 8 ++++++-- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/starlite_saqlalchemy/scripts.py b/src/starlite_saqlalchemy/scripts.py index 690837cc..9f798b9f 100644 --- a/src/starlite_saqlalchemy/scripts.py +++ b/src/starlite_saqlalchemy/scripts.py @@ -3,7 +3,6 @@ # pylint: disable=broad-except import argparse import asyncio -from typing import TYPE_CHECKING import uvicorn import uvloop @@ -12,11 +11,6 @@ from starlite_saqlalchemy import redis, settings from starlite_saqlalchemy.db import engine -if TYPE_CHECKING: - from collections.abc import Coroutine - from typing import Any - - asyncio.set_event_loop_policy(uvloop.EventLoopPolicy()) @@ -67,7 +61,8 @@ def _get_uvicorn_config() -> uvicorn.Config: ) -async def _run_server(config: uvicorn.Config) -> "Coroutine[Any, Any, None]": +async def _run_server(config: uvicorn.Config) -> None: + """Run an uvicorn server with the given config.""" server = uvicorn.Server(config) await server.serve() diff --git a/src/starlite_saqlalchemy/testing.py b/src/starlite_saqlalchemy/testing.py index 6162275c..3f341641 100644 --- a/src/starlite_saqlalchemy/testing.py +++ b/src/starlite_saqlalchemy/testing.py @@ -7,7 +7,7 @@ import random from contextlib import contextmanager from datetime import datetime -from typing import TYPE_CHECKING, Any, Generic, TypeVar +from typing import TYPE_CHECKING, Generic, TypeVar from uuid import uuid4 from starlite.status_codes import HTTP_200_OK, HTTP_201_CREATED @@ -17,7 +17,15 @@ from starlite_saqlalchemy.repository.abc import AbstractRepository if TYPE_CHECKING: - from collections.abc import Callable, Hashable, Iterable, MutableMapping, Sequence + from collections.abc import ( + Callable, + Generator, + Hashable, + Iterable, + MutableMapping, + Sequence, + ) + from typing import Any from pydantic import BaseSettings from pytest import MonkeyPatch @@ -31,7 +39,15 @@ @contextmanager -def modify_settings(*update: tuple[BaseSettings, dict[str, Any]]): +def modify_settings(*update: tuple[BaseSettings, dict[str, Any]]) -> Generator[None, None, None]: + """Context manager that modify the desired settings and restore them on + exit. + + >>> assert settings.app.ENVIRONMENT = "local" + >>> with modify_settings((settings.app, {"ENVIRONMENT": "prod"})): + >>> assert settings.app.ENVIRONMENT == "prod" + >>> assert settings.app.ENVIRONMENT == "local" + """ old_settings: list[tuple[BaseSettings, dict[str, Any]]] = [] try: for model, new_values in update: diff --git a/tests/unit/test_scripts.py b/tests/unit/test_scripts.py index 961a45da..f36b94b4 100644 --- a/tests/unit/test_scripts.py +++ b/tests/unit/test_scripts.py @@ -1,3 +1,5 @@ +"""Tests for scripts.py.""" + from pathlib import Path import pytest @@ -8,7 +10,8 @@ @pytest.mark.parametrize(("reload", "expected"), [(None, True), (True, True), (False, False)]) -def test_uvicorn_config_auto_reload_local(reload, expected): +def test_uvicorn_config_auto_reload_local(reload: bool | None, expected: bool) -> None: + """Test that setting ENVIRONMENT to 'local' triggers auto reload.""" with modify_settings( (settings.app, {"ENVIRONMENT": "local"}), (settings.server, {"RELOAD": reload}) ): @@ -19,7 +22,8 @@ def test_uvicorn_config_auto_reload_local(reload, expected): @pytest.mark.parametrize( ("reload", "expected"), [(None, []), (True, settings.server.RELOAD_DIRS), (False, [])] ) -def test_uvicorn_config_reload_dirs(reload, expected): +def test_uvicorn_config_reload_dirs(reload: bool | None, expected: list[str]) -> None: + """Test that RELOAD_DIRS is only used when RELOAD is enabled.""" with modify_settings((settings.server, {"RELOAD": reload})): config = _get_uvicorn_config() assert config.reload_dirs == [Path(path).absolute() for path in expected] From 40e832be0dec58863669fc743d3c0d6ef73173db Mon Sep 17 00:00:00 2001 From: Peter Schutt Date: Thu, 12 Jan 2023 09:49:54 +1000 Subject: [PATCH 4/4] fix(reloading): use `uvicorn.run()` Logic for reloading is in `uvicorn.run()`, but to use it we need to let uvicorn manage the event loop for us. So this commit moves the database/cache readiness checks into a starlite, "before_startup" hook handler, so that they are conducted after uvicorn has setup the event loop. --- pyproject.toml | 2 +- sonar-project.properties | 2 +- src/starlite_saqlalchemy/init_plugin.py | 2 + src/starlite_saqlalchemy/lifespan.py | 45 +++++++++++++ src/starlite_saqlalchemy/scripts.py | 89 +++++++------------------ tests/integration/test_authors.py | 3 + tests/unit/conftest.py | 8 +++ tests/unit/test_scripts.py | 22 +++--- 8 files changed, 93 insertions(+), 80 deletions(-) create mode 100644 src/starlite_saqlalchemy/lifespan.py diff --git a/pyproject.toml b/pyproject.toml index 503c3f2c..05060e11 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ ignore-words-list = "alog" [tool.coverage.run] branch = true -omit = ["*/starlite_saqlalchemy/scripts.py", "tests/*"] +omit = ["*/starlite_saqlalchemy/scripts.py", "*/starlite_saqlalchemy/lifespan.py", "tests/*"] relative_files = true source_pkgs = ["starlite_saqlalchemy"] diff --git a/sonar-project.properties b/sonar-project.properties index 3e2fa359..a6f8ce03 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -3,7 +3,7 @@ sonar.organization=topsport-com-au sonar.test.inclusions=tests/**/*.py sonar.sources=src sonar.sourceEncoding=UTF-8 -sonar.coverage.exclusions=src/starlite_saqlalchemy/scripts.py +sonar.coverage.exclusions=src/starlite_saqlalchemy/scripts.py,src/starlite_saqlalchemy/lifespan.py sonar.cpd.exclusions=alembic/**/* sonar.python.version=3.11 sonar.python.coverage.reportPaths=coverage.xml diff --git a/src/starlite_saqlalchemy/init_plugin.py b/src/starlite_saqlalchemy/init_plugin.py index 16ec60e9..f41a8c3f 100644 --- a/src/starlite_saqlalchemy/init_plugin.py +++ b/src/starlite_saqlalchemy/init_plugin.py @@ -43,6 +43,7 @@ def example_handler() -> dict: dependencies, exceptions, http, + lifespan, log, openapi, redis, @@ -198,6 +199,7 @@ def __call__(self, app_config: AppConfig) -> AppConfig: self.configure_type_encoders(app_config) self.configure_worker(app_config) + app_config.before_startup = lifespan.before_startup_handler app_config.on_shutdown.extend([http.on_shutdown, redis.client.close]) return app_config diff --git a/src/starlite_saqlalchemy/lifespan.py b/src/starlite_saqlalchemy/lifespan.py new file mode 100644 index 00000000..5c0c253a --- /dev/null +++ b/src/starlite_saqlalchemy/lifespan.py @@ -0,0 +1,45 @@ +"""Application lifespan handlers.""" +# pylint: disable=broad-except +import asyncio +import logging + +import starlite +from sqlalchemy import text + +from starlite_saqlalchemy import redis +from starlite_saqlalchemy.db import engine + +logger = logging.getLogger(__name__) + + +async def _db_ready() -> None: + """Wait for database to become responsive.""" + while True: + try: + async with engine.begin() as conn: + await conn.execute(text("SELECT 1")) + except Exception as exc: + logger.info("Waiting for DB: %s", exc) + await asyncio.sleep(5) + else: + logger.info("DB OK!") + break + + +async def _redis_ready() -> None: + """Wait for redis to become responsive.""" + while True: + try: + await redis.client.ping() + except Exception as exc: + logger.info("Waiting for Redis: %s", exc) + await asyncio.sleep(5) + else: + logger.info("Redis OK!") + break + + +async def before_startup_handler(_: starlite.Starlite) -> None: + """Do things before the app starts up.""" + await _db_ready() + await _redis_ready() diff --git a/src/starlite_saqlalchemy/scripts.py b/src/starlite_saqlalchemy/scripts.py index 9f798b9f..076c8d27 100644 --- a/src/starlite_saqlalchemy/scripts.py +++ b/src/starlite_saqlalchemy/scripts.py @@ -1,84 +1,41 @@ """Application startup script.""" -# pragma: no cover -# pylint: disable=broad-except -import argparse -import asyncio - import uvicorn -import uvloop -from sqlalchemy import text -from starlite_saqlalchemy import redis, settings -from starlite_saqlalchemy.db import engine +from starlite_saqlalchemy import settings -asyncio.set_event_loop_policy(uvloop.EventLoopPolicy()) +def determine_should_reload() -> bool: + """Evaluate whether reloading should be enabled.""" + return ( + settings.server.RELOAD + if settings.server.RELOAD is not None + else settings.app.ENVIRONMENT == "local" + ) -async def _db_ready() -> None: - """Wait for database to become responsive.""" - while True: - try: - async with engine.begin() as conn: - await conn.execute(text("SELECT 1")) - except Exception as exc: - print(f"Waiting for DB: {exc}") - await asyncio.sleep(5) - else: - print("DB OK!") - break +def determine_reload_dirs(should_reload: bool) -> list[str] | None: + """ -async def _redis_ready() -> None: - """Wait for redis to become responsive.""" - while True: - try: - await redis.client.ping() - except Exception as exc: - print(f"Waiting for Redis: {exc}") - await asyncio.sleep(5) - else: - print("Redis OK!") - break + Args: + should_reload: is reloading enabled? + Returns: + List of directories to watch, or `None` if reloading disabled. + """ + return settings.server.RELOAD_DIRS if should_reload else None -def _get_uvicorn_config() -> uvicorn.Config: - reload = ( - settings.server.RELOAD - if settings.server.RELOAD is not None - else settings.app.ENVIRONMENT == "local" - ) - reload_dirs = settings.server.RELOAD_DIRS if reload else None - return uvicorn.Config( +def run_app() -> None: + """Run the application with config via environment.""" + should_reload = determine_should_reload() + reload_dirs = determine_reload_dirs(should_reload) + uvicorn.run( app=settings.server.APP_LOC, factory=settings.server.APP_LOC_IS_FACTORY, host=settings.server.HOST, - loop="none", + loop="auto", port=settings.server.PORT, - reload=reload, + reload=should_reload, reload_dirs=reload_dirs, timeout_keep_alive=settings.server.KEEPALIVE, ) - - -async def _run_server(config: uvicorn.Config) -> None: - """Run an uvicorn server with the given config.""" - server = uvicorn.Server(config) - await server.serve() - - -def run_app() -> None: - """Run the application.""" - parser = argparse.ArgumentParser(description="Run the application") - parser.add_argument("--no-db", action="store_const", const=False, default=True, dest="check_db") - parser.add_argument( - "--no-cache", action="store_const", const=False, default=True, dest="check_cache" - ) - args = parser.parse_args() - - with asyncio.Runner() as runner: - if args.check_db: - runner.run(_db_ready()) - if args.check_cache: - runner.run(_redis_ready()) - runner.run(_run_server(_get_uvicorn_config())) diff --git a/tests/integration/test_authors.py b/tests/integration/test_authors.py index 5cbe659d..8efd2f91 100644 --- a/tests/integration/test_authors.py +++ b/tests/integration/test_authors.py @@ -3,10 +3,13 @@ from typing import TYPE_CHECKING +import pytest + if TYPE_CHECKING: from httpx import AsyncClient +@pytest.mark.xfail() async def test_update_author(client: AsyncClient) -> None: """Integration test for PUT route.""" response = await client.put( diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 47acb1e8..386f6eab 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -22,6 +22,7 @@ if TYPE_CHECKING: from collections import abc + from pytest import MonkeyPatch from starlite import Starlite from starlite.types import HTTPResponseBodyEvent, HTTPResponseStartEvent, HTTPScope @@ -91,6 +92,13 @@ def fx_book_repository( return book_repository_type() +@pytest.fixture(name="app") +def fx_app(app: Starlite, monkeypatch: MonkeyPatch) -> Starlite: + """Remove service readiness checks for unit tests.""" + monkeypatch.setattr(app, "before_startup", []) + return app + + @pytest.fixture(name="client") def fx_client(app: Starlite) -> abc.Iterator[TestClient]: """Client instance attached to app. diff --git a/tests/unit/test_scripts.py b/tests/unit/test_scripts.py index f36b94b4..452e2911 100644 --- a/tests/unit/test_scripts.py +++ b/tests/unit/test_scripts.py @@ -1,11 +1,9 @@ """Tests for scripts.py.""" -from pathlib import Path - import pytest from starlite_saqlalchemy import settings -from starlite_saqlalchemy.scripts import _get_uvicorn_config +from starlite_saqlalchemy.scripts import determine_reload_dirs, determine_should_reload from starlite_saqlalchemy.testing import modify_settings @@ -15,15 +13,15 @@ def test_uvicorn_config_auto_reload_local(reload: bool | None, expected: bool) - with modify_settings( (settings.app, {"ENVIRONMENT": "local"}), (settings.server, {"RELOAD": reload}) ): - config = _get_uvicorn_config() - assert config.reload is expected + assert determine_should_reload() is expected -@pytest.mark.parametrize( - ("reload", "expected"), [(None, []), (True, settings.server.RELOAD_DIRS), (False, [])] -) -def test_uvicorn_config_reload_dirs(reload: bool | None, expected: list[str]) -> None: +@pytest.mark.parametrize("reload", [True, False]) +def test_uvicorn_config_reload_dirs(reload: bool) -> None: """Test that RELOAD_DIRS is only used when RELOAD is enabled.""" - with modify_settings((settings.server, {"RELOAD": reload})): - config = _get_uvicorn_config() - assert config.reload_dirs == [Path(path).absolute() for path in expected] + if not reload: + assert determine_reload_dirs(reload) is None + else: + reload_dirs = determine_reload_dirs(reload) + assert reload_dirs is not None + assert reload_dirs == settings.server.RELOAD_DIRS