# Quality Checklist

Run every item when auditing an existing service or verifying a newly generated one. Mark each ✅ (pass) or ❌ (fail). Report all failures grouped by severity before declaring done.

---

## Severity Levels
- **BLOCKER** — deploy will fail, CI will fail, or data will be incorrect. Must fix before merge.
- **IMPORTANT** — significant quality/security issue. Should fix before merge.
- **MINOR** — style, naming, or cleanup. Fix before merge if time permits.

---

## 1. Entry Point & Startup

- [ ] **BLOCKER** `application.py` exists and `ddtrace.patch_all()` is the first executed statement, before any other imports.
- [ ] **BLOCKER** A `# noqa` comment follows the `ddtrace.patch_all()` line.
- [ ] **BLOCKER** `uwsgi-start.sh` exists and is referenced in `Dockerfile ENTRYPOINT`.
- [ ] **BLOCKER** `software-catalog.yaml` exists with `apiVersion: v3`, `owner: insights`, `tags: [public:false]`.
- [ ] **IMPORTANT** `SENTRY_DSN` raises `Exception` at startup if unset in QA or prod environments.
- [ ] **IMPORTANT** `application.py` imports handler modules after ddtrace patching (side-effect route registration).
- [ ] **MINOR** `dev.py` exists and runs Flask in debug mode on port 5000.

## 2. Configuration

- [ ] **BLOCKER** All route path strings are defined in `config.py` as module-level constants — no inline strings in `@app.route(...)`.
- [ ] **BLOCKER** `ENVIRONMENT` reads from `os.environ.get("Environment")` (capital E) — not lowercase.
- [ ] **IMPORTANT** `DB_URL = 'sqlite://'` when `ENVIRONMENT == TEST_ENVIRONMENT` — never connect to real Snowflake in tests.
- [ ] **IMPORTANT** `SNOWFLAKE_HOME` env var is set in the Dockerfile `deploy` stage to a writable directory.
- [ ] **IMPORTANT** `SNOWFLAKE_POOL_SIZE` equals uWSGI `--cheaper-initial` worker count (default: both are 15).
- [ ] **IMPORTANT** Dev reads from `os.environ`; QA/prod reads from AWS Secrets Manager (`secrets_manager.flask_ext`).
- [ ] **MINOR** `.env.shadow` exists (committed); `.env` is in `.gitignore` (never committed).

## 3. Handlers

- [ ] **BLOCKER** A global `@app.errorhandler(500)` exists in `handlers.py` and returns a JSON error via `flaskify(response.create_fatal_response(...))`.
- [ ] **BLOCKER** Health check endpoint exists at `config.HEALTH_CHECK_PATH` (e.g. `/hello/`) and returns `{"status": "ok"}`.
- [ ] **IMPORTANT** Handlers are thin — no SQL, no business logic, no direct DB calls in handler functions.
- [ ] **IMPORTANT** All handlers use `flaskify(response.Response(data))` for success, not raw `jsonify()`.
- [ ] **IMPORTANT** Handlers use `flaskify(response.create_error_response(...))` for expected errors (403, 404), not `abort()`.
- [ ] **MINOR** Health check path is excluded from owslogger access logs via `exclude_paths=[config.HEALTH_CHECK_PATH]` in `api.py`.

## 4. Schemas

- [ ] **IMPORTANT** All schemas inherit from `BaseSchema` (which sets `Meta.unknown = EXCLUDE`).
- [ ] **IMPORTANT** A base `RequestSchema` error handler maps marshmallow 422 → 500 (Orchard standard — never expose 400/422 to callers).
- [ ] **IMPORTANT** Schema files follow the inner `Request`/`Response` class pattern (one file per endpoint group).
- [ ] **IMPORTANT** Output field names use `data_key` to convert snake_case → camelCase in JSON responses.
- [ ] **MINOR** No Pydantic models in new code — marshmallow is the team standard.
- [ ] **MINOR** Mutable defaults use `load_default` (not `missing`) — marshmallow 3.x convention.

## 5. Database (Snowflake)

- [ ] **IMPORTANT** No ORM for Snowflake — raw SQL only. SQLAlchemy is connection-pool only.
- [ ] **IMPORTANT** SQL strings are module-level constants (not inline in function calls) OR loaded from `.sql` files.
- [ ] **IMPORTANT** `map_db_result_with_column_names()` is used to convert raw tuples to dicts.
- [ ] **MINOR** Long/complex queries are in `.sql` files, not embedded Python strings.
- [ ] **MINOR** Pool recycle is set to ~4h 55min (`4 * 55 * 60`) to recycle before Snowflake's idle timeout.

## 6. Redis / Caching

- [ ] **BLOCKER** `fakeredis` is in `requirements.txt` (production dependency, not dev-only).
- [ ] **IMPORTANT** `connectors/redis.py` falls back to `fakeredis.FakeStrictRedis()` when `REDIS_HOST` is unset — no conditional import that fails.
- [ ] **IMPORTANT** `@cache_in_redis` decorator fails silently on Redis errors (logs warning, proceeds with DB call).
- [ ] **IMPORTANT** Caching is in the logic layer, not the handler layer.
- [ ] **IMPORTANT** Health check does not open a new Redis connection per request — use the shared module-level `client`.

## 7. Logging

- [ ] **IMPORTANT** `owslogger.flask_logger.setup(...)` is called in `api.py` with `SERVICE_NAME`, `SERVICE_VERSION`, and `exclude_paths`.
- [ ] **IMPORTANT** `flask_request.setup(app, config.ENVIRONMENT, add_request_context=True)` is called in `api.py`.
- [ ] **IMPORTANT** No `logging.basicConfig()` calls — owslogger handles all logging configuration.
- [ ] **MINOR** Structured log calls use `app.logger.warning(...)` / `app.logger.exception(...)` (not `print()`).

## 8. Authentication & Security

- [ ] **IMPORTANT** No hardcoded API keys or secrets in source code.
- [ ] **IMPORTANT** If the service implements an access check, it uses the `@verify_profile` decorator pattern (not inline header parsing in handlers).
- [ ] **IMPORTANT** `verify_profile` returns `RESPONSE_FORBIDDEN` (403) for unauthorized callers — not 401.
- [ ] **MINOR** `.env.shadow` contains only placeholder values (e.g. `SENTRY_DSN=change_me`), not real credentials.

## 9. Tests

- [ ] **BLOCKER** Test files exist (`tests/unit/test_handlers.py` at minimum).
- [ ] **BLOCKER** `tests/unit/conftest.py` patches `Session.execute` at session scope so no Snowflake connection is opened.
- [ ] **IMPORTANT** Every handler has a test for: (a) success path, (b) 500 error path (logic raises exception).
- [ ] **IMPORTANT** Handler tests mock the logic layer (not the DB layer).
- [ ] **IMPORTANT** Marshmallow validation test: missing required field → HTTP 500 (not 400/422).
- [ ] **IMPORTANT** `fakeredis` is used in unit tests (no patching needed — `REDIS_HOST` is unset in test env).
- [ ] **MINOR** Tests are organized in classes (`TestMyHandler`, `TestMyLogic`).
- [ ] **MINOR** `@pytest.mark.parametrize` used for input/output combinations.

## 10. Dockerfile

- [ ] **BLOCKER** `deploy` stage does NOT copy `tests/` directory.
- [ ] **BLOCKER** `deploy` stage uses explicit `COPY` (not `ADD . /var/app`).
- [ ] **BLOCKER** `ENTRYPOINT ["/uwsgi-start.sh"]` — the script must exist in the repo.
- [ ] **IMPORTANT** `deploy` stage creates `uwsgi` user with `-s /bin/false` and `chown -R uwsgi:uwsgi /var/app/`.
- [ ] **IMPORTANT** `ENV SNOWFLAKE_HOME=/var/app/.snowflake` is set in the `deploy` stage.
- [ ] **IMPORTANT** `pr_tests` stage exists for running tests in CI.
- [ ] **MINOR** Base image is `python:3.11-slim-bullseye`.

## 11. Jenkinsfile

- [ ] **BLOCKER** `software-catalog.yaml` is validated by `datadogSoftwareCatalogValidate()` in the Build Test and Scan parallel stage.
- [ ] **BLOCKER** `QA_DEPLOYMENT_ROLE` and `PROD_DEPLOYMENT_ROLE` are non-empty strings (not `''`).
- [ ] **BLOCKER** `ECR_ACCOUNT_ID`, `QA_ACCOUNT_ID`, `PROD_ACCOUNT_ID` are non-empty strings.
- [ ] **IMPORTANT** `VULNERABILITIES_TO_IGNORE = []` — no CVEs suppressed without investigation and a comment.
- [ ] **IMPORTANT** `imageTag: env.GIT_COMMIT` — not `env.BUILD_NUMBER`.
- [ ] **IMPORTANT** `dockerBuildTarget: 'deploy'` — not the default (which would build the full image).
- [ ] **IMPORTANT** `datadogSoftwareCatalogPublish()` called after prod deploy.
- [ ] **IMPORTANT** `disableConcurrentBuilds()` is in the `options {}` block.
- [ ] **MINOR** Slack channel is `#insights-engineering` (or the team's specific channel).
- [ ] **MINOR** `issueCommentTrigger('.*retest this please.*')` is in `triggers {}`.

## 12. Dependencies

- [ ] **BLOCKER** `requirements.txt` starts with `-i https://pypi.theorchard.io/pypi`.
- [ ] **BLOCKER** All versions in `requirements.txt` are exact pins — no `>=`, `~=`, or `^`.
- [ ] **IMPORTANT** `fakeredis` is in `requirements.txt` (not `requirements-dev.txt`).
- [ ] **IMPORTANT** Test-only libraries (`pytest`, `flake8`, `mypy`, `black`, `isort`) are in `requirements-dev.txt` only.
- [ ] **IMPORTANT** `arq`, `flower`, and other unused/monitoring-only packages are not in production deps.
- [ ] **MINOR** Python version matches between `Dockerfile FROM python:3.11-*`, `.python-version`, and `mypy.ini python_version`.

## 13. Project Structure

- [ ] **IMPORTANT** `connectors/` contains only connection management — no business logic.
- [ ] **IMPORTANT** `logic/` functions receive plain Python values, not Flask `request` objects.
- [ ] **IMPORTANT** `schemas/` contains only marshmallow serialization — no DB queries.
- [ ] **MINOR** `handlers.py` / `*_handlers.py` naming convention (not `views.py`).
- [ ] **MINOR** `__init__.py` files are empty (no re-exports that create circular imports).
- [ ] **MINOR** No `manual-testing-plan.md` or similar ephemeral docs tracked in the repo root.