forgejo_admin/pal-e-mail
Archived
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

feat: scaffold pal-e-mail service (Phase 1) #2

Merged
forgejo_admin merged 1 commit from 1-phase1-service-scaffold into main 2026-03-22 15:41:36 +00:00
Conversation 2 Commits 1 Files changed 15 +358
forgejo_admin commented 2026-03-22 09:43:25 +00:00
Owner
Copy link

Summary

Scaffolds the pal-e-mail FastAPI service with all foundational files: project config, SQLAlchemy models, Alembic migrations, Dockerfile, Woodpecker CI pipeline, and a healthz endpoint. Multi-tenant email logging by project field, using gmail-sdk-ldraney from Forgejo PyPI.

Changes

  • pyproject.toml -- project metadata, dependencies (fastapi, sqlalchemy, alembic, gmail-sdk-ldraney), dev tools (pytest, ruff)
  • src/pal_e_mail/__init__.py -- package init
  • src/pal_e_mail/config.py -- Settings dataclass reading PAL_E_MAIL_* env vars
  • src/pal_e_mail/database.py -- SQLAlchemy engine, session, DeclarativeBase
  • src/pal_e_mail/models.py -- EmailLog model with project-based multi-tenancy, EmailStatus enum
  • src/pal_e_mail/main.py -- FastAPI app with /healthz endpoint
  • Dockerfile -- 3-stage build (base/builder/runtime), Forgejo PyPI extra-index-url
  • .woodpecker.yaml -- CI pipeline: lint + test with postgres service, kaniko build-and-push to Harbor
  • alembic.ini + alembic/env.py + alembic/script.py.mako -- Alembic config with env var override
  • alembic/versions/001_email_log.py -- Initial migration creating email_log table
  • tests/test_healthz.py -- Healthz endpoint test
  • .gitignore -- Python/ruff/pytest ignores

Test Plan

  • ruff check . passes clean
  • ruff format --check . passes clean
  • pytest tests/test_healthz.py -- 1 passed (0.21s)
  • CI pipeline runs on PR

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive

Related

  • Closes #1
  • Plan: plan-pal-e-mail Phase 1
## Summary Scaffolds the pal-e-mail FastAPI service with all foundational files: project config, SQLAlchemy models, Alembic migrations, Dockerfile, Woodpecker CI pipeline, and a healthz endpoint. Multi-tenant email logging by project field, using gmail-sdk-ldraney from Forgejo PyPI. ## Changes - `pyproject.toml` -- project metadata, dependencies (fastapi, sqlalchemy, alembic, gmail-sdk-ldraney), dev tools (pytest, ruff) - `src/pal_e_mail/__init__.py` -- package init - `src/pal_e_mail/config.py` -- Settings dataclass reading `PAL_E_MAIL_*` env vars - `src/pal_e_mail/database.py` -- SQLAlchemy engine, session, DeclarativeBase - `src/pal_e_mail/models.py` -- EmailLog model with project-based multi-tenancy, EmailStatus enum - `src/pal_e_mail/main.py` -- FastAPI app with `/healthz` endpoint - `Dockerfile` -- 3-stage build (base/builder/runtime), Forgejo PyPI extra-index-url - `.woodpecker.yaml` -- CI pipeline: lint + test with postgres service, kaniko build-and-push to Harbor - `alembic.ini` + `alembic/env.py` + `alembic/script.py.mako` -- Alembic config with env var override - `alembic/versions/001_email_log.py` -- Initial migration creating email_log table - `tests/test_healthz.py` -- Healthz endpoint test - `.gitignore` -- Python/ruff/pytest ignores ## Test Plan - [x] `ruff check .` passes clean - [x] `ruff format --check .` passes clean - [x] `pytest tests/test_healthz.py` -- 1 passed (0.21s) - [ ] CI pipeline runs on PR ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #1 - Plan: `plan-pal-e-mail` Phase 1
FastAPI service with EmailLog model, Alembic migrations, Dockerfile,
Woodpecker CI pipeline, and healthz endpoint. Multi-tenant by project
field. Uses gmail-sdk-ldraney from Forgejo PyPI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-03-22 09:43:52 +00:00
Author
Owner
Copy link

Review: PASS

Reviewed all 15 files (358 additions). No issues found.

Verified:

  • ruff check . -- clean
  • ruff format --check . -- clean
  • pytest tests/test_healthz.py -- 1 passed
  • No secrets committed (alembic.ini uses localhost defaults only)
  • No extraneous files (.current-issue, .venv excluded by .gitignore)
  • Dockerfile 3-stage build is correct (base/builder/runtime)
  • Woodpecker CI pipeline matches minio-api pattern (custom clone, postgres service, kaniko push)
  • Alembic migration matches model definition (EmailLog with all columns + emailstatus enum)
  • Config uses PAL_E_MAIL_* env var prefix consistently

No fixes needed. Ready for merge.

## Review: PASS Reviewed all 15 files (358 additions). No issues found. **Verified:** - `ruff check .` -- clean - `ruff format --check .` -- clean - `pytest tests/test_healthz.py` -- 1 passed - No secrets committed (alembic.ini uses localhost defaults only) - No extraneous files (.current-issue, .venv excluded by .gitignore) - Dockerfile 3-stage build is correct (base/builder/runtime) - Woodpecker CI pipeline matches minio-api pattern (custom clone, postgres service, kaniko push) - Alembic migration matches model definition (EmailLog with all columns + emailstatus enum) - Config uses `PAL_E_MAIL_*` env var prefix consistently No fixes needed. Ready for merge.
forgejo_admin deleted branch 1-phase1-service-scaffold 2026-03-22 15:41:36 +00:00
forgejo_admin commented 2026-03-22 15:42:18 +00:00
Author
Owner
Copy link

PR #2 Review

DOMAIN REVIEW

Tech stack: Python 3.12 / FastAPI / SQLAlchemy 2.0 / Alembic / Woodpecker CI / Kaniko / Harbor

Port consistency -- EXPOSE 8000, CMD --port 8000, uvicorn binding 0.0.0.0:8000. All consistent. Pass.

Forgejo PyPI extra-index-url -- Present in both Dockerfile (line 9) and .woodpecker.yaml (line 32). Matches the minio-api CI pattern where git is also installed in the test step for the private PyPI dependency. Pass.

alembic env.py -- Reads PAL_E_MAIL_DATABASE_URL from environment (line 14), overrides sqlalchemy.url from alembic.ini when set. Correct pattern. Pass.

Model vs migration -- All 10 columns match between models.py and 001_email_log.py: types, lengths, nullability, indexes, and server defaults are consistent. One minor divergence: the model uses default=EmailStatus.sent (Python-side ORM default) while the migration uses server_default="sent" (database-side default). Both are functionally correct -- the DB will enforce the default on raw INSERTs, and SQLAlchemy will set it on ORM creates. Not a blocker but the model should ideally also declare server_default for parity.

Secrets -- No API keys, passwords, tokens, or .env files committed. Harbor credentials use from_secret. Postgres creds in CI service and alembic.ini are local-dev/test-only defaults. Clean.

CI pipeline -- Follows platform patterns: alpine/git clone block, postgres service container, lint + test step, kaniko build-and-push gated to main push. Matches basketball-api and minio-api reference pipelines.

BLOCKERS

None.

The scaffold delivers exactly one endpoint (/healthz) and one test covering it. There are no CRUD endpoints, no user input handling, no auth paths, and no secrets in code. All blocker criteria pass.

NITS

  1. Missing [build-system] in pyproject.toml -- PEP 517/518 requires a [build-system] table. Without it, pip install . falls back to legacy setuptools and emits a deprecation warning. Add:

    [build-system]
requires = ["setuptools>=68.0"]
build-backend = "setuptools.backends._legacy:_Backend"

    Or use the standard:

    [build-system]
requires = ["setuptools>=68.0", "wheel"]
build-backend = "setuptools.build_meta"

  2. Module-level engine creation -- database.py creates the SQLAlchemy engine at import time via settings.database_url, and config.py reads env vars at class-definition time (not instantiation). This means PAL_E_MAIL_DATABASE_URL must be set before any import of pal_e_mail.database or pal_e_mail.models. The current test (test_healthz.py) only imports main.py which does not import database.py, so it works. But future tests that touch models will need PAL_E_MAIL_DATABASE_URL set before import or a conftest.py that patches the engine. Worth noting for Phase 2.

  3. Model default vs server_default -- models.py line 27 uses default=EmailStatus.sent (Python-side), but the migration uses server_default="sent" (DB-side). Consider adding server_default="sent" to the model's mapped_column() as well, so the model is a complete description of the DB schema.

  4. Missing path.exclude for ArgoCD -- basketball-api and mcd-tracker-api have path.exclude: ["k8s/.argocd-source-*"] in their when block to prevent CI from retriggering on ArgoCD source annotations. Not needed yet (no k8s/ dir), but worth adding when the deployment manifests land.

  5. httpx duplicated in dependencies -- httpx>=0.28 appears in both dependencies and [project.optional-dependencies].dev. The dev entry is redundant since it is already a production dependency.

SOP COMPLIANCE

  • Branch named after issue (1-phase1-service-scaffold references issue #1)
  • PR body has: Summary, Changes, Test Plan, Related
  • Related references plan slug (plan-pal-e-mail Phase 1)
  • Tests exist (1 test, covers the only endpoint)
  • No secrets committed
  • No unnecessary file changes (all files are scaffold-relevant)
  • Commit messages are descriptive

PROCESS OBSERVATIONS

  • Deployment frequency: Clean scaffold sets up CI pipeline from day one. Once merged, main-branch pushes will auto-build and push to Harbor. Good DORA foundation.
  • Change failure risk: Low. This is a greenfield scaffold with no existing consumers. The only runtime concern is the alembic upgrade head in the CMD -- if the DB is unreachable at startup, the container will crash-loop. A future phase should add a health-check wait or init container for migration.
  • Documentation: README exists but is minimal. Phase 2+ should expand it with local dev setup, env var reference, and API docs.

VERDICT: APPROVED

## PR #2 Review ### DOMAIN REVIEW **Tech stack**: Python 3.12 / FastAPI / SQLAlchemy 2.0 / Alembic / Woodpecker CI / Kaniko / Harbor **Port consistency** -- EXPOSE 8000, CMD `--port 8000`, uvicorn binding `0.0.0.0:8000`. All consistent. Pass. **Forgejo PyPI extra-index-url** -- Present in both `Dockerfile` (line 9) and `.woodpecker.yaml` (line 32). Matches the `minio-api` CI pattern where `git` is also installed in the test step for the private PyPI dependency. Pass. **alembic env.py** -- Reads `PAL_E_MAIL_DATABASE_URL` from environment (line 14), overrides `sqlalchemy.url` from `alembic.ini` when set. Correct pattern. Pass. **Model vs migration** -- All 10 columns match between `models.py` and `001_email_log.py`: types, lengths, nullability, indexes, and server defaults are consistent. One minor divergence: the model uses `default=EmailStatus.sent` (Python-side ORM default) while the migration uses `server_default="sent"` (database-side default). Both are functionally correct -- the DB will enforce the default on raw INSERTs, and SQLAlchemy will set it on ORM creates. Not a blocker but the model should ideally also declare `server_default` for parity. **Secrets** -- No API keys, passwords, tokens, or `.env` files committed. Harbor credentials use `from_secret`. Postgres creds in CI service and `alembic.ini` are local-dev/test-only defaults. Clean. **CI pipeline** -- Follows platform patterns: `alpine/git` clone block, postgres service container, lint + test step, kaniko build-and-push gated to `main` push. Matches `basketball-api` and `minio-api` reference pipelines. ### BLOCKERS None. The scaffold delivers exactly one endpoint (`/healthz`) and one test covering it. There are no CRUD endpoints, no user input handling, no auth paths, and no secrets in code. All blocker criteria pass. ### NITS 1. **Missing `[build-system]` in `pyproject.toml`** -- PEP 517/518 requires a `[build-system]` table. Without it, `pip install .` falls back to legacy setuptools and emits a deprecation warning. Add: ```toml [build-system] requires = ["setuptools>=68.0"] build-backend = "setuptools.backends._legacy:_Backend" ``` Or use the standard: ```toml [build-system] requires = ["setuptools>=68.0", "wheel"] build-backend = "setuptools.build_meta" ``` 2. **Module-level engine creation** -- `database.py` creates the SQLAlchemy engine at import time via `settings.database_url`, and `config.py` reads env vars at class-definition time (not instantiation). This means `PAL_E_MAIL_DATABASE_URL` must be set before _any_ import of `pal_e_mail.database` or `pal_e_mail.models`. The current test (`test_healthz.py`) only imports `main.py` which does not import `database.py`, so it works. But future tests that touch models will need `PAL_E_MAIL_DATABASE_URL` set before import or a `conftest.py` that patches the engine. Worth noting for Phase 2. 3. **Model `default` vs `server_default`** -- `models.py` line 27 uses `default=EmailStatus.sent` (Python-side), but the migration uses `server_default="sent"` (DB-side). Consider adding `server_default="sent"` to the model's `mapped_column()` as well, so the model is a complete description of the DB schema. 4. **Missing `path.exclude` for ArgoCD** -- `basketball-api` and `mcd-tracker-api` have `path.exclude: ["k8s/.argocd-source-*"]` in their `when` block to prevent CI from retriggering on ArgoCD source annotations. Not needed yet (no `k8s/` dir), but worth adding when the deployment manifests land. 5. **`httpx` duplicated in dependencies** -- `httpx>=0.28` appears in both `dependencies` and `[project.optional-dependencies].dev`. The dev entry is redundant since it is already a production dependency. ### SOP COMPLIANCE - [x] Branch named after issue (`1-phase1-service-scaffold` references issue #1) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related references plan slug (`plan-pal-e-mail` Phase 1) - [x] Tests exist (1 test, covers the only endpoint) - [x] No secrets committed - [x] No unnecessary file changes (all files are scaffold-relevant) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - **Deployment frequency**: Clean scaffold sets up CI pipeline from day one. Once merged, main-branch pushes will auto-build and push to Harbor. Good DORA foundation. - **Change failure risk**: Low. This is a greenfield scaffold with no existing consumers. The only runtime concern is the `alembic upgrade head` in the CMD -- if the DB is unreachable at startup, the container will crash-loop. A future phase should add a health-check wait or init container for migration. - **Documentation**: README exists but is minimal. Phase 2+ should expand it with local dev setup, env var reference, and API docs. ### VERDICT: APPROVED
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/pal-e-mail!2
No description provided.