Skip to content

Commit

Permalink
Use Flask custom CLI commands for custom scripts (#138)
Browse files Browse the repository at this point in the history
## Changes
* Use Flask custom CLI commands for custom scripts
* Remove main() entry point from create_user_csv service
* Remove script_utils
* Rename api.route package to api.api, and organize all things related
to blueprints together:
   * X_blueprint.py
   * X_commands.py
   * X_routes.py
   * X_schemas.py
* Rewrite test_create_user_csv to use test_cli_runner rather than only
test the service
* Add migration to cascade on delete
* Rewrite test_create_user_csv to truncate the user table rather than
requiring an isolated db schema
* Remove now unused isolated_db fixture
* Remove unused transaction rollback functionality in db_session test
fixture
* Rename /user REST resource to /users

Other changes
* Organize Makefile commands:
   * move setup-local to top
   * group openapi-spec with other CLI commands)
* Install pytest-lazy-fixture dev dependency
* Move empty_schema fixture to test_migrations, the only place it's used

## Context
This PR consolidates the way we run the API server and the way we run
background scripts to both use Flask's create_app factory. This removes
the need to have two separate main() entrypoints, two separate ways to
initialize the database, logger, etc. To do this, we leverage [Flask's
built-in method of creating custom CLI
commands](https://flask.palletsprojects.com/en/2.2.x/cli/#custom-commands)
which itself is based on the [click
library](https://click.palletsprojects.com/en/8.1.x/arguments/), a
popular library for building Python command line applications that's
more powerful than `argparse`. This allows us to remove the script_utils
file.

This PR also renames the `api.route` package to `api.api` since we want
to add "commands" to the user blueprint, so "route" is no longer an
appropriate name that applies to everything in the blueprint. We could
also consider other names like `api.components` or `api.blueprints`,
and/or we could consider renaming the top level source code folder to
something like `src` rather than `api`.

Finally, we made test_create_user_csv test things at using the
test_cli_runner to be closer to the way we test routes using the
test_client. This approach has more end-to-end test coverage.

Other tangential changes include making REST resources use plural names
to follow REST resource naming conventions.
  • Loading branch information
lorenyu authored Feb 21, 2023
1 parent f964468 commit 000bced
Show file tree
Hide file tree
Showing 39 changed files with 277 additions and 548 deletions.
44 changes: 26 additions & 18 deletions app/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ else
PY_RUN_CMD := docker-compose run $(DOCKER_EXEC_ARGS) --rm $(APP_NAME) poetry run
endif

FLASK_CMD := $(PY_RUN_CMD) flask --env-file local.env

##################################################
# Local Development Environment Setup
##################################################

setup-local:
# Configure poetry to use virtualenvs in the project directory
poetry config virtualenvs.in-project true

# Install dependencies
poetry install --no-root

##################################################
# API Build & Run
##################################################
Expand All @@ -46,7 +59,6 @@ start:
run-logs: start
docker-compose logs --follow --no-color $(APP_NAME)


init: build init-db

clean-volumes: ## Remove project docker volumes (which includes the DB state)
Expand All @@ -57,13 +69,6 @@ stop:

check: format-check lint test

# Set init-db as pre-requisite since there seems to be a race condition
# where the DB can't yet receive connections if it's starting from a
# clean state (e.g. after make stop, make clean-volumes, make openapi-spec)
openapi-spec: init-db ## Generate OpenAPI spec
$(PY_RUN_CMD) flask spec --format yaml --output ./openapi.yml


##################################################
# DB & migrations
##################################################
Expand Down Expand Up @@ -168,11 +173,21 @@ lint-security: # https://bandit.readthedocs.io/en/latest/index.html


##################################################
# Scripts
# CLI Commands
##################################################

create-user-csv:
$(PY_RUN_CMD) create-user-csv
cmd: ## Run Flask app CLI command (Run `make cli args="--help"` to see list of CLI commands)
$(FLASK_CMD) $(args)

cmd-user-create-csv: ## Create a CSV of the useres in the database (Run `make cli-user-create-csv args="--help"` to see the command's options)
$(FLASK_CMD) user create-csv $(args)

# Set init-db as pre-requisite since there seems to be a race condition
# where the DB can't yet receive connections if it's starting from a
# clean state (e.g. after make stop, make clean-volumes, make openapi-spec)
openapi-spec: init-db ## Generate OpenAPI spec
$(FLASK_CMD) spec --format yaml --output ./openapi.yml


##################################################
# Miscellaneous Utilities
Expand All @@ -184,10 +199,3 @@ login: start ## Start shell in running container
# Pauses for 5 seconds
sleep-5:
sleep 5

setup-local:
# Configure poetry to use virtualenvs in the project directory
poetry config virtualenvs.in-project true

# Install dependencies
poetry install --no-root
File renamed without changes.
4 changes: 2 additions & 2 deletions app/api/route/healthcheck.py → app/api/api/healthcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
from werkzeug.exceptions import ServiceUnavailable

import api.adapters.db.flask_db as flask_db
from api.route import response
from api.route.schemas import request_schema
from api.api import response
from api.api.schemas import request_schema

logger = logging.getLogger(__name__)

Expand Down
2 changes: 1 addition & 1 deletion app/api/route/response.py → app/api/api/response.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import dataclasses
from typing import Optional

from api.api.schemas import response_schema
from api.db.models.base import Base
from api.route.schemas import response_schema


@dataclasses.dataclass
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from apiflask import fields

from api.route.schemas import request_schema
from api.api.schemas import request_schema


class ValidationErrorSchema(request_schema.OrderedSchema):
Expand Down
10 changes: 10 additions & 0 deletions app/api/api/users/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from api.api.users.user_blueprint import user_blueprint

# import user_commands module to register the CLI commands on the user_blueprint
import api.api.users.user_commands # noqa: F401 E402 isort:skip

# import user_commands module to register the API routes on the user_blueprint
import api.api.users.user_routes # noqa: F401 E402 isort:skip


__all__ = ["user_blueprint"]
3 changes: 3 additions & 0 deletions app/api/api/users/user_blueprint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from apiflask import APIBlueprint

user_blueprint = APIBlueprint("user", __name__, tag="User", cli_group="user")
34 changes: 34 additions & 0 deletions app/api/api/users/user_commands.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import logging
import os.path as path
from typing import Optional

import click

import api.adapters.db as db
import api.adapters.db.flask_db as flask_db
import api.services.users as user_service
from api.api.users.user_blueprint import user_blueprint
from api.util.datetime_util import utcnow

logger = logging.getLogger(__name__)

user_blueprint.cli.help = "User commands"


@user_blueprint.cli.command("create-csv", help="Create a CSV of all users and their roles")
@flask_db.with_db_session
@click.option(
"--dir",
default=".",
help="Directory to save output file in. Can be an S3 path (e.g. 's3://bucketname/folder/') Defaults to current directory ('.').",
)
@click.option(
"--filename",
default=None,
help="Filename to save output file as. Defaults to '[timestamp]-user-roles.csv.",
)
def create_csv(db_session: db.Session, dir: str, filename: Optional[str]) -> None:
if filename is None:
filename = utcnow().strftime("%Y-%m-%d-%H-%M-%S") + "-user-roles.csv"
filepath = path.join(dir, filename)
user_service.create_user_csv(db_session, filepath)
44 changes: 12 additions & 32 deletions app/api/route/user_route.py → app/api/api/users/user_routes.py
Original file line number Diff line number Diff line change
@@ -1,44 +1,34 @@
import logging
from typing import Any

from apiflask import APIBlueprint

import api.adapters.db as db
import api.adapters.db.flask_db as flask_db
import api.api.response as response
import api.api.users.user_schemas as user_schemas
import api.services.users as user_service
import api.services.users as users
from api.api.users.user_blueprint import user_blueprint
from api.auth.api_key_auth import api_key_auth
from api.db.models.user_models import User
from api.route import response
from api.route.schemas import user_schemas
from api.services import users

logger = logging.getLogger(__name__)


user_blueprint = APIBlueprint("user", __name__, tag="User")


@user_blueprint.post("/v1/user")
@user_blueprint.post("/v1/users")
@user_blueprint.input(user_schemas.UserSchema)
@user_blueprint.output(user_schemas.UserSchema, status_code=201)
@user_blueprint.auth_required(api_key_auth)
@flask_db.with_db_session
def user_post(db_session: db.Session, user_params: users.CreateUserParams) -> dict:
"""
POST /v1/user
POST /v1/users
"""
user = user_service.create_user(db_session, user_params)

logger.info(
"Successfully inserted user",
extra=get_user_log_params(user),
)
print("Successfully inserted user", get_user_log_params(user))

logger.info("Successfully inserted user", extra=get_user_log_params(user))
return response.ApiResponse(message="Success", data=user).asdict()


@user_blueprint.patch("/v1/user/<uuid:user_id>")
@user_blueprint.patch("/v1/users/<uuid:user_id>")
# Allow partial updates. partial=true means requests that are missing
# required fields will not be rejected.
# https://marshmallow.readthedocs.io/en/stable/quickstart.html#partial-loading
Expand All @@ -50,29 +40,19 @@ def user_patch(
db_session: db.Session, user_id: str, patch_user_params: users.PatchUserParams
) -> dict:
user = user_service.patch_user(db_session, user_id, patch_user_params)

logger.info(
"Successfully patched user",
extra=get_user_log_params(user),
)

logger.info("Successfully patched user", extra=get_user_log_params(user))
return response.ApiResponse(message="Success", data=user).asdict()


@user_blueprint.get("/v1/user/<uuid:user_id>")
@user_blueprint.get("/v1/users/<uuid:user_id>")
@user_blueprint.output(user_schemas.UserSchema)
@user_blueprint.auth_required(api_key_auth)
@flask_db.with_db_session
def user_get(db_session: db.Session, user_id: str) -> dict:
user = user_service.get_user(db_session, user_id)

logger.info(
"Successfully fetched user",
extra=get_user_log_params(user),
)

logger.info("Successfully fetched user", extra=get_user_log_params(user))
return response.ApiResponse(message="Success", data=user).asdict()


def get_user_log_params(user: User) -> dict[str, Any]:
return {"user_id": user.id}
return {"user.id": user.id}
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
from apiflask import fields
from marshmallow import fields as marshmallow_fields

from api.api.schemas import request_schema
from api.db.models import user_models
from api.route.schemas import request_schema

##############
# Role Models
##############


class RoleSchema(request_schema.OrderedSchema):
Expand All @@ -20,11 +16,6 @@ class RoleSchema(request_schema.OrderedSchema):
# will always be a nested fields of the API user


##############
# User Models
##############


class UserSchema(request_schema.OrderedSchema):
id = fields.UUID(dump_only=True)
first_name = fields.String(metadata={"description": "The user's first name"}, required=True)
Expand Down
6 changes: 3 additions & 3 deletions app/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
import api.adapters.db.flask_db as flask_db
import api.logging
import api.logging.flask_logger as flask_logger
from api.api.healthcheck import healthcheck_blueprint
from api.api.schemas import response_schema
from api.api.users import user_blueprint
from api.auth.api_key_auth import User, get_app_security_scheme
from api.route.healthcheck import healthcheck_blueprint
from api.route.schemas import response_schema
from api.route.user_route import user_blueprint

logger = logging.getLogger(__name__)

Expand Down
30 changes: 30 additions & 0 deletions app/api/db/migrations/versions/2023_02_21_cascade_on_delete.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
"""cascade on delete
Revision ID: 9fe657340f70
Revises: 4ff1160282d1
Create Date: 2023-02-21 18:16:56.052679
"""
from alembic import op

# revision identifiers, used by Alembic.
revision = "9fe657340f70"
down_revision = "4ff1160282d1"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_constraint("role_user_id_user_fkey", "role", type_="foreignkey")
op.create_foreign_key(
op.f("role_user_id_user_fkey"), "role", "user", ["user_id"], ["id"], ondelete="CASCADE"
)
# ### end Alembic commands ###


def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_constraint(op.f("role_user_id_user_fkey"), "role", type_="foreignkey")
op.create_foreign_key("role_user_id_user_fkey", "role", "user", ["user_id"], ["id"])
# ### end Alembic commands ###
6 changes: 4 additions & 2 deletions app/api/db/models/user_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ class User(Base, IdMixin, TimestampMixin):
date_of_birth: date = Column(Date, nullable=False)
is_active: bool = Column(Boolean, nullable=False)

roles: list["Role"] = relationship("Role", back_populates="user", order_by="Role.type")
roles: list["Role"] = relationship(
"Role", back_populates="user", cascade="all, delete", order_by="Role.type"
)


class Role(Base, TimestampMixin):
__tablename__ = "role"
user_id: Mapped[UUID] = Column(
postgresql.UUID(as_uuid=True), ForeignKey("user.id"), primary_key=True
postgresql.UUID(as_uuid=True), ForeignKey("user.id", ondelete="CASCADE"), primary_key=True
)

# Set native_enum=False to use store enum values as VARCHAR/TEXT
Expand Down
21 changes: 0 additions & 21 deletions app/api/route/route_utils.py

This file was deleted.

Empty file removed app/api/scripts/__init__.py
Empty file.
Empty file removed app/api/scripts/util/__init__.py
Empty file.
41 changes: 0 additions & 41 deletions app/api/scripts/util/script_util.py

This file was deleted.

Loading

0 comments on commit 000bced

Please sign in to comment.