Skip to content

Commit

Permalink
🎨(api) version and reorganize api structure
Browse files Browse the repository at this point in the history
- Version API with `v1`
- Rename `routes` directory to `routers`
- Move models and schemas under specific dirs
- Move Pydantic models from `api/models.py` to `schemas.py`
- Move `api/auth.py` to `auth.py`
  • Loading branch information
wilbrdt committed Nov 6, 2024
1 parent d8e146d commit 0372fa8
Show file tree
Hide file tree
Showing 28 changed files with 92 additions and 60 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 20 additions & 9 deletions src/app/mork/api/__init__.py
Original file line number Diff line number Diff line change
@@ -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"])

Expand All @@ -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)
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""API routes related to application health checking."""
"""API health router."""

import logging
from enum import Enum
Expand All @@ -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__)

Expand Down
9 changes: 9 additions & 0 deletions src/app/mork/api/v1/__init__.py
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions src/app/mork/api/v1/routers/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Mork API v1 routers."""
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
"""API routes related to tasks."""
"""API tasks router."""

import logging
from typing import Union

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,
Expand Down
2 changes: 1 addition & 1 deletion src/app/mork/api/auth.py → src/app/mork/auth.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/app/mork/celery/tasks/deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down
4 changes: 2 additions & 2 deletions src/app/mork/celery/tasks/emailing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down
File renamed without changes.
1 change: 1 addition & 0 deletions src/app/mork/factories/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Mork factories."""
4 changes: 2 additions & 2 deletions src/app/mork/factories.py → src/app/mork/factories/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import factory

from mork import models
from mork.models.tasks import EmailStatus


class EmailStatusFactory(factory.alchemy.SQLAlchemyModelFactory):
Expand All @@ -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")
1 change: 1 addition & 0 deletions src/app/mork/migrations/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions src/app/mork/models/__init__.py
Original file line number Diff line number Diff line change
@@ -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
}
18 changes: 3 additions & 15 deletions src/app/mork/models.py → src/app/mork/models/tasks.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down
1 change: 1 addition & 0 deletions src/app/mork/schemas/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Mork schemas."""
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Mork models."""
"""Mork tasks schemas."""

from enum import Enum, unique
from typing import Literal
Expand Down
4 changes: 2 additions & 2 deletions src/app/mork/tests/api/test_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/app/mork/tests/api/v1/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Mork API v1 tests."""
1 change: 1 addition & 0 deletions src/app/mork/tests/api/v1/routers/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Mork API v1 routers tests."""
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

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

Expand All @@ -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
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/app/mork/tests/celery/tasks/test_deletion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/app/mork/tests/celery/tasks/test_emailing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion src/app/mork/tests/fixtures/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
File renamed without changes.

0 comments on commit 0372fa8

Please sign in to comment.