diff --git a/CHANGELOG.md b/CHANGELOG.md index f44859e..f73c0b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ## Changed - Add a dry-run parameter to the task creation API (defaults to True) +- Introduce API versioning with `v1` namespace ## Fixed diff --git a/src/app/mork/api/__init__.py b/src/app/mork/api/__init__.py index 7bfe06f..75a3efa 100644 --- a/src/app/mork/api/__init__.py +++ b/src/app/mork/api/__init__.py @@ -1,26 +1,28 @@ """Main module for Mork API.""" from functools import lru_cache -from typing import Dict, List, Union from urllib.parse import urlparse import sentry_sdk from fastapi import FastAPI from fastapi.concurrency import asynccontextmanager +from sentry_sdk.integrations.fastapi import FastApiIntegration +from sentry_sdk.integrations.starlette import StarletteIntegration from mork import __version__ -from mork.api.routes import health, tasks +from mork.api.health import router as health_router +from mork.api.v1 import app as v1 from mork.conf import settings -from mork.database import get_engine +from mork.db import get_engine @lru_cache(maxsize=None) -def get_health_check_routes() -> List: +def get_health_check_routes() -> list: """Return the health check routes.""" - return [route.path for route in health.router.routes] + return [route.path for route in health_router.routes] -def filter_transactions(event: Dict, hint) -> Union[Dict, None]: # noqa: ARG001 +def filter_transactions(event: dict, hint) -> dict | None: # noqa: ARG001 """Filter transactions for Sentry.""" url = urlparse(event["request"]["url"]) @@ -39,17 +41,26 @@ async def lifespan(app: FastAPI): # noqa: ARG001 if settings.SENTRY_DSN is not None: sentry_sdk.init( dsn=settings.SENTRY_DSN, + enable_tracing=True, traces_sample_rate=settings.SENTRY_API_TRACES_SAMPLE_RATE, release=__version__, environment=settings.SENTRY_EXECUTION_ENVIRONMENT, max_breadcrumbs=50, before_send_transaction=filter_transactions, + integrations=[ + StarletteIntegration(), + FastApiIntegration(), + ], ) yield engine.dispose() -app = FastAPI(lifespan=lifespan) -app.include_router(health.router) -app.include_router(tasks.router) +app = FastAPI(title="Mork", lifespan=lifespan) + +# Health checks +app.include_router(health_router) + +# Mount v1 API +app.mount("/v1", v1) diff --git a/src/app/mork/api/routes/health.py b/src/app/mork/api/health.py similarity index 90% rename from src/app/mork/api/routes/health.py rename to src/app/mork/api/health.py index 8422ce8..26d8c05 100644 --- a/src/app/mork/api/routes/health.py +++ b/src/app/mork/api/health.py @@ -1,4 +1,4 @@ -"""API routes related to application health checking.""" +"""API health router.""" import logging from enum import Enum @@ -8,8 +8,8 @@ from pydantic import BaseModel from sqlalchemy.orm import Session -from mork.database import get_session -from mork.database import is_alive as is_db_alive +from mork.db import get_session +from mork.db import is_alive as is_db_alive logger = logging.getLogger(__name__) diff --git a/src/app/mork/api/v1/__init__.py b/src/app/mork/api/v1/__init__.py new file mode 100644 index 0000000..3c97cda --- /dev/null +++ b/src/app/mork/api/v1/__init__.py @@ -0,0 +1,9 @@ +"""Mork API v1.""" + +from fastapi import FastAPI + +from mork.api.v1.routers import tasks + +app = FastAPI(title="Mork API (v1)") + +app.include_router(tasks.router) diff --git a/src/app/mork/api/v1/routers/__init__.py b/src/app/mork/api/v1/routers/__init__.py new file mode 100644 index 0000000..129177a --- /dev/null +++ b/src/app/mork/api/v1/routers/__init__.py @@ -0,0 +1 @@ +"""Mork API v1 routers.""" diff --git a/src/app/mork/api/routes/tasks.py b/src/app/mork/api/v1/routers/tasks.py similarity index 93% rename from src/app/mork/api/routes/tasks.py rename to src/app/mork/api/v1/routers/tasks.py index 9e9af2f..990a6e8 100644 --- a/src/app/mork/api/routes/tasks.py +++ b/src/app/mork/api/v1/routers/tasks.py @@ -1,4 +1,4 @@ -"""API routes related to tasks.""" +"""API tasks router.""" import logging from typing import Union @@ -6,8 +6,8 @@ from celery.result import AsyncResult from fastapi import APIRouter, Body, Depends, Response, status -from mork.api.auth import authenticate_api_key -from mork.api.models import ( +from mork.auth import authenticate_api_key +from mork.schemas.tasks import ( TASK_TYPE_TO_FUNC, DeleteInactiveUsers, DeleteUser, diff --git a/src/app/mork/api/auth.py b/src/app/mork/auth.py similarity index 91% rename from src/app/mork/api/auth.py rename to src/app/mork/auth.py index 0bf7ac2..b512f25 100644 --- a/src/app/mork/api/auth.py +++ b/src/app/mork/auth.py @@ -1,4 +1,4 @@ -"""Main module for Mork API authentication.""" +"""Main module for Mork authentication.""" from fastapi import HTTPException, Security, status from fastapi.security import APIKeyHeader diff --git a/src/app/mork/celery/tasks/deletion.py b/src/app/mork/celery/tasks/deletion.py index 02b4059..c6f74db 100644 --- a/src/app/mork/celery/tasks/deletion.py +++ b/src/app/mork/celery/tasks/deletion.py @@ -8,11 +8,11 @@ from mork.celery.celery_app import app from mork.conf import settings -from mork.database import MorkDB +from mork.db import MorkDB from mork.edx.mysql import crud from mork.edx.mysql.database import OpenEdxMySQLDB from mork.exceptions import UserDeleteError -from mork.models import EmailStatus +from mork.models.tasks import EmailStatus logger = getLogger(__name__) diff --git a/src/app/mork/celery/tasks/emailing.py b/src/app/mork/celery/tasks/emailing.py index 0794d03..95c4ffc 100644 --- a/src/app/mork/celery/tasks/emailing.py +++ b/src/app/mork/celery/tasks/emailing.py @@ -8,12 +8,12 @@ from mork.celery.celery_app import app from mork.conf import settings -from mork.database import MorkDB +from mork.db import MorkDB from mork.edx.mysql import crud from mork.edx.mysql.database import OpenEdxMySQLDB from mork.exceptions import EmailAlreadySent, EmailSendError from mork.mail import send_email -from mork.models import EmailStatus +from mork.models.tasks import EmailStatus logger = getLogger(__name__) diff --git a/src/app/mork/database.py b/src/app/mork/db.py similarity index 100% rename from src/app/mork/database.py rename to src/app/mork/db.py diff --git a/src/app/mork/factories/__init__.py b/src/app/mork/factories/__init__.py new file mode 100644 index 0000000..03b9f19 --- /dev/null +++ b/src/app/mork/factories/__init__.py @@ -0,0 +1 @@ +"""Mork factories.""" diff --git a/src/app/mork/factories.py b/src/app/mork/factories/tasks.py similarity index 82% rename from src/app/mork/factories.py rename to src/app/mork/factories/tasks.py index de3d70e..866d324 100644 --- a/src/app/mork/factories.py +++ b/src/app/mork/factories/tasks.py @@ -2,7 +2,7 @@ import factory -from mork import models +from mork.models.tasks import EmailStatus class EmailStatusFactory(factory.alchemy.SQLAlchemyModelFactory): @@ -11,7 +11,7 @@ class EmailStatusFactory(factory.alchemy.SQLAlchemyModelFactory): class Meta: """Factory configuration.""" - model = models.EmailStatus + model = EmailStatus email = factory.Faker("email") sent_date = factory.Faker("date_time") diff --git a/src/app/mork/migrations/env.py b/src/app/mork/migrations/env.py index 8aa3d6a..fe48fc1 100644 --- a/src/app/mork/migrations/env.py +++ b/src/app/mork/migrations/env.py @@ -9,6 +9,7 @@ # Nota bene: be sure to import all models that need to be migrated here from mork.models import Base +from mork.models.tasks import EmailStatus # this is the Alembic Config object, which provides # access to the values within the .ini file in use. diff --git a/src/app/mork/models/__init__.py b/src/app/mork/models/__init__.py new file mode 100644 index 0000000..5219d18 --- /dev/null +++ b/src/app/mork/models/__init__.py @@ -0,0 +1,17 @@ +"""Mork models.""" + +from sqlalchemy.orm import DeclarativeBase + + +class Base(DeclarativeBase): + """Base class for all models in the database.""" + + filtered_attrs = [] + + def safe_dict(self): + """Return a dictionary representation of the model.""" + return { + c.name: getattr(self, c.name) + for c in self.__table__.columns + if c.name not in self.filtered_attrs + } diff --git a/src/app/mork/models.py b/src/app/mork/models/tasks.py similarity index 52% rename from src/app/mork/models.py rename to src/app/mork/models/tasks.py index fc5d0c2..f0d81c0 100644 --- a/src/app/mork/models.py +++ b/src/app/mork/models/tasks.py @@ -1,25 +1,13 @@ -"""Mork models.""" +"""Mork tasks models.""" import datetime import uuid from sqlalchemy import DateTime, String from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column +from sqlalchemy.orm import Mapped, mapped_column - -class Base(DeclarativeBase): - """Base class for all models in the database.""" - - filtered_attrs = [] - - def safe_dict(self): - """Return a dictionary representation of the model.""" - return { - c.name: getattr(self, c.name) - for c in self.__table__.columns - if c.name not in self.filtered_attrs - } +from mork.models import Base class EmailStatus(Base): diff --git a/src/app/mork/schemas/__init__.py b/src/app/mork/schemas/__init__.py new file mode 100644 index 0000000..c08a532 --- /dev/null +++ b/src/app/mork/schemas/__init__.py @@ -0,0 +1 @@ +"""Mork schemas.""" diff --git a/src/app/mork/api/models.py b/src/app/mork/schemas/tasks.py similarity index 98% rename from src/app/mork/api/models.py rename to src/app/mork/schemas/tasks.py index 6280bb9..a73907a 100644 --- a/src/app/mork/api/models.py +++ b/src/app/mork/schemas/tasks.py @@ -1,4 +1,4 @@ -"""Mork models.""" +"""Mork tasks schemas.""" from enum import Enum, unique from typing import Literal diff --git a/src/app/mork/tests/api/test_health.py b/src/app/mork/tests/api/test_health.py index 4ead160..bcc32f1 100644 --- a/src/app/mork/tests/api/test_health.py +++ b/src/app/mork/tests/api/test_health.py @@ -14,13 +14,13 @@ async def test_api_health_lbheartbeat(http_client): @pytest.mark.anyio async def test_api_health_heartbeat(db_session, http_client, monkeypatch): """Test the heartbeat healthcheck.""" - monkeypatch.setattr("mork.database.get_session", db_session) + monkeypatch.setattr("mork.db.get_session", db_session) response = await http_client.get("/__heartbeat__") assert response.status_code == 200 assert response.json() == {"database": "ok"} - monkeypatch.setattr("mork.api.routes.health.is_db_alive", lambda x: False) + monkeypatch.setattr("mork.api.health.is_db_alive", lambda x: False) response = await http_client.get("/__heartbeat__") assert response.status_code == 500 diff --git a/src/app/mork/tests/api/v1/__init__.py b/src/app/mork/tests/api/v1/__init__.py new file mode 100644 index 0000000..8344e21 --- /dev/null +++ b/src/app/mork/tests/api/v1/__init__.py @@ -0,0 +1 @@ +"""Mork API v1 tests.""" diff --git a/src/app/mork/tests/api/v1/routers/__init__.py b/src/app/mork/tests/api/v1/routers/__init__.py new file mode 100644 index 0000000..a1a0200 --- /dev/null +++ b/src/app/mork/tests/api/v1/routers/__init__.py @@ -0,0 +1 @@ +"""Mork API v1 routers tests.""" diff --git a/src/app/mork/tests/api/test_tasks.py b/src/app/mork/tests/api/v1/routers/test_tasks.py similarity index 81% rename from src/app/mork/tests/api/test_tasks.py rename to src/app/mork/tests/api/v1/routers/test_tasks.py index 355216f..bfdd4b2 100644 --- a/src/app/mork/tests/api/test_tasks.py +++ b/src/app/mork/tests/api/v1/routers/test_tasks.py @@ -11,9 +11,9 @@ async def test_tasks_auth(http_client: AsyncClient): """Test required authentication for tasks endpoints.""" # FastAPI returns a 403 error (instead of a 401 error) if no API token is given # see https://github.com/tiangolo/fastapi/discussions/9130 - assert (await http_client.post("/tasks/")).status_code == 403 - assert (await http_client.options("/tasks/")).status_code == 403 - assert (await http_client.get("/tasks/status/1234")).status_code == 403 + assert (await http_client.post("/v1/tasks/")).status_code == 403 + assert (await http_client.options("/v1/tasks/")).status_code == 403 + assert (await http_client.get("/v1/tasks/status/1234")).status_code == 403 @pytest.mark.anyio @@ -30,7 +30,7 @@ async def test_tasks_auth(http_client: AsyncClient): ) @pytest.mark.parametrize( "tasks_endpoint", - ["/tasks/", "/tasks"], + ["/v1/tasks/", "/v1/tasks"], ) async def test_create_task( http_client: AsyncClient, auth_headers: dict, body_params: str, tasks_endpoint: str @@ -41,7 +41,7 @@ async def test_create_task( celery_task.delay.return_value.task_id = "1234" with patch.dict( - "mork.api.tasks.TASK_TYPE_TO_FUNC", {body_params["type"]: celery_task} + "mork.api.v1.tasks.TASK_TYPE_TO_FUNC", {body_params["type"]: celery_task} ): response = await http_client.post( tasks_endpoint, headers=auth_headers, json=body_params @@ -66,8 +66,8 @@ async def test_create_task_invalid_type(http_client: AsyncClient, auth_headers: """Test creating a task with an invalid type.""" # Without a type - with patch.dict("mork.api.tasks.TASK_TYPE_TO_FUNC"): - response = await http_client.post("/tasks/", headers=auth_headers) + with patch.dict("mork.api.v1.tasks.TASK_TYPE_TO_FUNC"): + response = await http_client.post("/v1/tasks/", headers=auth_headers) assert response.status_code == 422 @@ -77,9 +77,9 @@ async def test_create_task_invalid_type(http_client: AsyncClient, auth_headers: celery_task = Mock() celery_task.delay.return_value.task_id = "1234" - with patch.dict("mork.api.tasks.TASK_TYPE_TO_FUNC"): + with patch.dict("mork.api.v1.tasks.TASK_TYPE_TO_FUNC"): response = await http_client.post( - "/tasks/", headers=auth_headers, json=mock_task_create + "/v1/tasks/", headers=auth_headers, json=mock_task_create ) assert response.status_code == 422 @@ -100,9 +100,9 @@ async def test_create_task_missing_param( celery_task = Mock() celery_task.delay.return_value.task_id = "1234" - with patch.dict("mork.api.tasks.TASK_TYPE_TO_FUNC", {task_type: celery_task}): + with patch.dict("mork.api.v1.tasks.TASK_TYPE_TO_FUNC", {task_type: celery_task}): response = await http_client.post( - "/tasks/", headers=auth_headers, json=mock_task_create + "/v1/tasks/", headers=auth_headers, json=mock_task_create ) response_data = response.json() @@ -113,7 +113,7 @@ async def test_create_task_missing_param( @pytest.mark.anyio @pytest.mark.parametrize( "tasks_endpoint", - ["/tasks/", "/tasks"], + ["/v1/tasks/", "/v1/tasks"], ) async def test_get_available_tasks( http_client: AsyncClient, auth_headers: dict, tasks_endpoint: str @@ -138,9 +138,9 @@ async def test_get_task_status(http_client: AsyncClient, auth_headers: dict): celery_result = Mock(task_id) celery_result(task_id).state = "SUCCESS" - with patch("mork.api.tasks.AsyncResult", celery_result): + with patch("mork.api.v1.tasks.AsyncResult", celery_result): response = await http_client.get( - f"/tasks/status/{task_id}", + f"/v1/tasks/status/{task_id}", headers=auth_headers, ) response_data = response.json() diff --git a/src/app/mork/tests/celery/tasks/test_deletion.py b/src/app/mork/tests/celery/tasks/test_deletion.py index 6e36c3e..931db9e 100644 --- a/src/app/mork/tests/celery/tasks/test_deletion.py +++ b/src/app/mork/tests/celery/tasks/test_deletion.py @@ -17,8 +17,8 @@ from mork.edx.mysql import crud from mork.edx.mysql.factories.auth import EdxAuthUserFactory from mork.exceptions import UserDeleteError -from mork.factories import EmailStatusFactory -from mork.models import EmailStatus +from mork.factories.tasks import EmailStatusFactory +from mork.models.tasks import EmailStatus def test_delete_inactive_users(edx_mysql_db, monkeypatch): diff --git a/src/app/mork/tests/celery/tasks/test_emailing.py b/src/app/mork/tests/celery/tasks/test_emailing.py index 1836b53..d8723ac 100644 --- a/src/app/mork/tests/celery/tasks/test_emailing.py +++ b/src/app/mork/tests/celery/tasks/test_emailing.py @@ -13,7 +13,7 @@ ) from mork.edx.mysql.factories.auth import EdxAuthUserFactory from mork.exceptions import EmailAlreadySent, EmailSendError -from mork.factories import EmailStatusFactory +from mork.factories.tasks import EmailStatusFactory def test_warn_inactive_users(edx_mysql_db, monkeypatch): diff --git a/src/app/mork/tests/fixtures/db.py b/src/app/mork/tests/fixtures/db.py index da024d8..6de1834 100644 --- a/src/app/mork/tests/fixtures/db.py +++ b/src/app/mork/tests/fixtures/db.py @@ -10,7 +10,7 @@ from mork.edx.mysql.database import OpenEdxMySQLDB from mork.edx.mysql.factories.base import Session, engine from mork.edx.mysql.models.base import Base as EdxBase -from mork.models import Base +from mork.models.tasks import Base @pytest.fixture diff --git a/src/app/mork/tests/models/__init__.py b/src/app/mork/tests/models/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/app/mork/tests/test_models.py b/src/app/mork/tests/models/test_tasks.py similarity index 85% rename from src/app/mork/tests/test_models.py rename to src/app/mork/tests/models/test_tasks.py index aa06c04..5329e8e 100644 --- a/src/app/mork/tests/test_models.py +++ b/src/app/mork/tests/models/test_tasks.py @@ -1,6 +1,6 @@ """Tests of the Mork models.""" -from mork.factories import EmailStatusFactory +from mork.factories.tasks import EmailStatusFactory def test_models_user_safe_dict(db_session): diff --git a/src/app/mork/tests/api/test_auth.py b/src/app/mork/tests/test_auth.py similarity index 93% rename from src/app/mork/tests/api/test_auth.py rename to src/app/mork/tests/test_auth.py index 8237872..2871f25 100644 --- a/src/app/mork/tests/api/test_auth.py +++ b/src/app/mork/tests/test_auth.py @@ -3,7 +3,7 @@ import pytest from fastapi import HTTPException, status -from mork.api.auth import authenticate_api_key +from mork.auth import authenticate_api_key from mork.conf import settings diff --git a/src/app/mork/tests/test_database.py b/src/app/mork/tests/test_db.py similarity index 100% rename from src/app/mork/tests/test_database.py rename to src/app/mork/tests/test_db.py