Currently this is built out to use SQLAlchemy + Alembic to handle most operations with the Postgres database along with FactoryBoy+Faker to generate the test data.

Consider what alternative options could be available that might be:

• Simpler
• DB language agnostic

see #67 (comment)

I noticed we have make test and make test-coverage but not make test-watch. Could be useful for developer productivity

Context

make openapi-spec will generate the openapi.yml file from the schema files, but this is currently manually run, so it can get out of sync

Options

1. have a CI check that runs make openapi-spec and if it changes the file (e.g. the git index isn't clean) then fail the CI check
2. have a workflow that runs make openapi-spec on PRs and if there are any changes automatically commit and push the changes

Context

Looks like installing postgresql fixes it

https://stackoverflow.com/questions/11618898/pg-config-executable-not-found

This is draft idea for discussion.

Goal

Record the operation (batch job or API POST/PATCH/PUT) that created or last modified a row in some way in the database. This is often helpful when troubleshooting or debugging something.

Inspiration:

• created_at and updated_at track when a row was created or modified (TimestampMixin), but not what or who did it
• in pfml, some tables have a reference to import_log to track which batch job created or updated each row

Logs could answer this too, but it's not always easy to get from logs (especially in bulk), and it needs attention to have the right logging calls.

Proposal

Add created_mutation_id and updated_mutation_id columns, both FKs to a new table mutation:

Examples

Let's say the address table has these columns. It looks like this:

The mutation table has:

This shows that address 100 was:

• Created at 04:30 by batch job "address.import"
• Later on at 11:30 updated by an API POST by user 33445566

While address 101 was created by the same batch, and not modified since.

Out of scope

• Tracking all changes. Only creation and the most recent modification. It's too much to track all changes in the db for most use cases.

Context

template-application-flask still doesn't have CI. We want to add a ci-app.yml that runs the checks for the flask application. This CI should be able to be used on any project that uses the template.

See this github thread

Certain core folders like /api/adapters aren't meant to be modified. If they are modified, then future platform updates will be harder to pull in. Let platform team know before modifying, and consider putting the needed enhancement into the platform itself.

And add some tests too

Context

current folder structure is /app/api/...
let's rename to /app/lib/... or something else that can apply to both api code and background jobs

Once we do this, we can rename /lib/route to /lib/api since the stuff in the route package is really stuff that's API specific (api schemas, api handlers/controller layer)

Per discussion here: #51 (comment)
and here: https://nava.slack.com/archives/C03G1SWD9H7/p1671033815911879

We want to avoid using dicts in our service methods.

Context

the database commands in the makefile could use more comments and some additional documentation in the docs folder to explain how to use them and when to use them

i also couldn't find any instructions on how to start the database from scratch i.e. deleting the volume, which would be useful especially for template development

Once #3 has been done, go through and add additional API examples for other non-POST endpoints.

Context

poetry is being used as the package management tool, it'd be good to retroactively document this decision as an ADR

References

some references from a quick googling, but may find additional resources from internal slack

• https://www.reddit.com/r/Python/comments/limd9t/poetry_vs_pipenv_vs_piptools_what_do_you_use/
• https://medium.com/opendoor-labs/our-python-monorepo-d34028f2b6fa
• https://remaster.com/blog/pip-pipenv-poetry-comparison
• https://news.ycombinator.com/item?id=26093926

Needs refinement, but the idea is to separate out project specific packages from core packages to solidify hexagonal architecture and make it harder to add business logic to low level adapters, etc

Copy the WIC MT demo project over as-is.
https://github.com/navapbc/wic-mt-demo-project-mock-api

Our current setup has validation occuring in OpenAPI/Swagger, and then validation occuring via Pydantic models we define. Flask requires an openapi spec, but openapi can't do many validations and is clunky in a lot of ways to setup.

A lot of this ends up duplicated. Investigate ways of doing this validation differently.

A few leads:

• Use pydantic to generate the OpenAPI spec: https://github.com/kuimono/openapi-schema-pydantic
• Drop flask for FastAPI: https://fastapi.tiangolo.com/ - seems to solve this problem BUT would be scrapping most of our API approach (FastAPI actually has a template library: https://github.com/tiangolo/full-stack-fastapi-postgresql)
• https://apiflask.com/ which only looks to require a few small changes to get working from an existing Flask app / is just a wrapper around Flask.

Context

we've been discussing best practices around software architecture, creating service objects to encapsulate business logic, separating out domain layer, we want to document and implement examples of this practice

References

🔒 Server Side Software Design Guidelines

See https://lwd.atlassian.net/wiki/spaces/API/pages/157417509/API+server+framework

https://www.docker.com/blog/announcing-compose-v2-general-availability/

• https://python-poetry.org/blog/announcing-poetry-1.2.0/
• https://python-poetry.org/docs/master/managing-dependencies/

Context

Consider dbmate which is simpler to understand, and has no dependencies on Python or SQLAlchemy

see https://nava.slack.com/archives/C03G1SWD9H7/p1663861150188399

Could be nice convenience to add decorators that automatically create a new session in a context manager (and optionally start transaction)

Design options

could be two decorators with_db_session and with_db_session_transaction or could be one decorator with a parameter
with_db_session(start_transaction: bool)

Add an option in db module to connect to postgres db using IAM rather than with username/password
Use this as the default option
Ideally keep username/password option as an option for projects that don't need complexity of IAM auth

Considerations

Need to figure out how to test locally

Dependencies

This might depend on the aurora db template-infra work

Right now create_user_csv accesses S3 and local file storage directly but I think we should use the adapter pattern and have a module adapters/storage.py

See slack thread

Quick google shows a bunch of options, but not sure which is best:

• flask-rest-api
• flassger
• flask-openapi3
• apiflask

Should add tests/api/test_db.py to test db module in isolation

Add CI check to flake8 checks for cyclomatic complexity
Consider using https://libraries.io/pypi/mccabe or radon (bias towards mccabe on initial skim)

Make sure the user service module doesn't depend on the controller layer.

• Does not take an ApiContext as input
• db objects (eg User) as output, instead of UserResponse

After #2 - go through and remove the WIC MT specific naming and implementation including:

• Documentation
• API implementation
• Script implementation

Note this should make an equivalent, but not fully formed implementation (eg. missing thorough examples).

I wonder if validate_match functions are necessary or if we can rely on built in python operators

Context

PR #21 adds CI, but it doesn't seem to add type checking, so this ticket will add that

it'd be nice to see coverage stats on PRs
one idea is to have a github action workflow that adds a comment with coverage stats
another idea is to have the ci tests add show stats in the "step summary" (see https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#adding-a-job-summary)

Context

https://nava.slack.com/archives/C03G1SWD9H7/p1663856080445169

Notes

• also include things like constraint naming conventions (https://stackoverflow.com/questions/4107915/postgresql-default-constraint-names/4108266#4108266)

Context

This is important to ensure that it works with template-infra which sets that env var in ECS

DB env vars are currently prefixed as POSTGRES_* but I think DB_* I prefer names that represent the role/purpose rather than the language/tech

🔒 https://nava.slack.com/archives/C03G1SWD9H7/p1671137100224249

see #57 (comment)

Context

In the main comment thread of navapbc/wic-mt-demo-project-mock-api#20 we discussed not having ORM be in the default template.

Context

In order to productionize the current Dockerfile, we need to:

• use non-root user (see slack thread)
  • we don't need to make this an argument, we could probably hardcode it
• reduce the size of the built image by only copying over the necessary files (see how PFML does it)

References

• PFML Dockerfile