Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add db-migrate to container path to meet infra template requirements #176

Merged
merged 7 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ CMD ["poetry", "run", "python", "-m", "src"]

FROM base AS release
ARG RUN_USER
USER ${RUN_USER}
WORKDIR /app

# TODO(https://github.com/navapbc/template-application-flask/issues/23) Productionize the Docker image
Expand Down Expand Up @@ -95,9 +94,15 @@ RUN poetry install --no-root --only main
# or the application will not build
RUN poetry build --format wheel && poetry run pip install 'dist/template_application_flask-0.1.0-py3-none-any.whl'

# Add project's virtual env to the PATH so we can directly run poetry scripts
# defiend in pyproject.toml
ENV PATH="/app/.venv/bin:$PATH"

# Set the host to 0.0.0.0 to make the server available external
# to the Docker container that it's running in.
ENV HOST=0.0.0.0

USER ${RUN_USER}

# Run the application.
CMD ["poetry", "run", "python", "-m", "src"]
2 changes: 1 addition & 1 deletion app/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ alembic_config := ./src/db/migrations/alembic.ini
alembic_cmd := $(PY_RUN_CMD) alembic --config $(alembic_config)

db-upgrade: ## Apply pending migrations to db
$(PY_RUN_CMD) db-migrate-up
$(PY_RUN_CMD) db-migrate

db-downgrade: ## Rollback last migration in db
$(PY_RUN_CMD) db-migrate-down
Expand Down
2 changes: 1 addition & 1 deletion app/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ build-backend = "poetry.core.masonry.api"

[tool.poetry.scripts]
app-start = "src.__main__:main"
db-migrate-up = "src.db.migrations.run:up"
db-migrate = "src.db.migrations.run:up"
db-migrate-down = "src.db.migrations.run:down"
db-migrate-down-all = "src.db.migrations.run:downall"

Expand Down
4 changes: 0 additions & 4 deletions app/src/adapters/db/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
This module contains the DBClient class, which is used to manage database connections

For usage information look at the package docstring in __init__.py

This module also contains lower level connection related functions such as
make_connection_uri that can be used outside of the application context such as for
database migrations.
"""
import abc
import logging
Expand Down
35 changes: 0 additions & 35 deletions app/src/adapters/db/clients/postgres_client.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
import urllib.parse
from typing import Any

import boto3
Expand Down Expand Up @@ -122,40 +121,6 @@ def generate_iam_auth_token(aws_region: str, host: str, port: int, user: str) ->
return token


def make_connection_uri(config: PostgresDBConfig) -> str:
"""Construct PostgreSQL connection URI

More details at:
https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
"""
host = config.host
db_name = config.name
username = config.username
password = urllib.parse.quote(config.password) if config.password else None
schema = config.db_schema
port = config.port

netloc_parts = []

if username and password:
netloc_parts.append(f"{username}:{password}@")
elif username:
netloc_parts.append(f"{username}@")
elif password:
netloc_parts.append(f":{password}@")

netloc_parts.append(host)

if port:
netloc_parts.append(f":{port}")

netloc = "".join(netloc_parts)

uri = f"postgresql://{netloc}/{db_name}?options=-csearch_path={schema}"

return uri


def verify_ssl(connection_info: Any) -> None:
"""Verify that the database connection is encrypted and log a warning if not."""
if connection_info.ssl_in_use:
Expand Down
67 changes: 10 additions & 57 deletions app/src/db/migrations/env.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,12 @@
import logging
import sys
from typing import Any

import alembic.context as context
import sqlalchemy
from alembic import context

# Alembic cli seems to reset the path on load causing issues with local module imports.
# Workaround is to force set the path to the current run directory (top level src folder)
# See database migrations section in `./database/database-migrations.md` for details about running migrations.
sys.path.insert(0, ".") # noqa: E402

# Load env vars before anything further
from src.util.local import load_local_env_vars # noqa: E402 isort:skip

load_local_env_vars()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do the env vars get loaded locally? I thought that was done when running Flask commands, but this doesn't look to run via Flask.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question. i have to confess that was prioritizing getting database integration working, so i intentionally let running on host machine break. i was gonna work on that soon with #136

i'm currently thinking of having developers install direnv, and having an app/.envrc file that looks like

source local.env

and then whenever you cd into app direnv will automatically get all the env vars from local.env into your shell, and when you cd out of the directory direnv will automatically unload those env vars.

the benefit of this is that local developers can also override vars in local.env without risking checking those in by adding the overrides to .envrc e.g.

source local.env
export DB_HOST=localhost

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I actually probably already broke running host machine with PR #172 , so it was probably already broken before this PR)


from src.adapters.db.clients.postgres_client import make_connection_uri # noqa: E402 isort:skip
from src.adapters.db.clients.postgres_config import get_db_config # noqa: E402 isort:skip
from src.db.models import metadata # noqa: E402 isort:skip
import src.logging # noqa: E402 isort:skip
import src.adapters.db as db
import src.logging
from src.db.models import metadata

# this is the Alembic Config object, which provides
# access to the values within the .ini file in use.
Expand All @@ -29,13 +17,6 @@
# Initialize logging
with src.logging.init("migrations"):

if not config.get_main_option("sqlalchemy.url"):
uri = make_connection_uri(get_db_config())

# Escape percentage signs in the URI.
# https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option
config.set_main_option("sqlalchemy.url", uri.replace("%", "%%"))

# add your model's MetaData object here
# for 'autogenerate' support
# from myapp import mymodel
Expand All @@ -59,33 +40,6 @@ def include_object(
else:
return True

def run_migrations_offline() -> None:
"""Run migrations in 'offline' mode.

This configures the context with just a URL
and not an Engine, though an Engine is acceptable
here as well. By skipping the Engine creation
we don't even need a DBAPI to be available.

Calls to context.execute() here emit the given string to the
script output.

"""

url = config.get_main_option("sqlalchemy.url")
context.configure(
url=url,
target_metadata=target_metadata,
literal_binds=True,
dialect_opts={"paramstyle": "named"},
include_schemas=False,
include_object=include_object,
compare_type=True,
)

with context.begin_transaction():
context.run_migrations()

def run_migrations_online() -> None:
"""Run migrations in 'online' mode.

Expand All @@ -94,10 +48,9 @@ def run_migrations_online() -> None:

"""

url = config.get_main_option("sqlalchemy.url", "SQLAlchemy URL not set")
connectable = sqlalchemy.create_engine(url)
db_client = db.PostgresDBClient()

with connectable.connect() as connection:
with db_client.get_connection() as connection:
context.configure(
connection=connection,
target_metadata=target_metadata,
Expand All @@ -108,7 +61,7 @@ def run_migrations_online() -> None:
with context.begin_transaction():
context.run_migrations()

if context.is_offline_mode():
run_migrations_offline()
else:
run_migrations_online()
# No need to support running migrations in offline mode.
# When running locally we have the local containerized database.
# When running in the cloud we'll have the actual cloud database.
run_migrations_online()
3 changes: 2 additions & 1 deletion app/src/db/migrations/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import os
from typing import Optional

import alembic.command as command
import alembic.script as script
import sqlalchemy
from alembic import command, script
from alembic.config import Config
from alembic.operations.ops import MigrationScript
from alembic.runtime import migration
Expand Down
41 changes: 3 additions & 38 deletions app/tests/src/adapters/db/clients/test_postgres_client.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import logging # noqa: B1
from itertools import product
import logging

import pytest

from src.adapters.db.clients.postgres_client import (
get_connection_parameters,
make_connection_uri,
verify_ssl,
)
from src.adapters.db.clients.postgres_config import PostgresDBConfig, get_db_config
from src.adapters.db.clients.postgres_client import get_connection_parameters, verify_ssl
from src.adapters.db.clients.postgres_config import get_db_config


class DummyConnectionInfo:
Expand Down Expand Up @@ -43,36 +38,6 @@ def test_verify_ssl_not_in_use(caplog):
assert caplog.records[0].levelname == "WARNING"


@pytest.mark.parametrize(
"username_password_port,expected",
zip(
# Test all combinations of username, password, and port
product(["testuser", ""], ["testpass", None], [5432]),
[
"postgresql://testuser:testpass@localhost:5432/dbname?options=-csearch_path=public",
"postgresql://testuser@localhost:5432/dbname?options=-csearch_path=public",
"postgresql://:testpass@localhost:5432/dbname?options=-csearch_path=public",
"postgresql://localhost:5432/dbname?options=-csearch_path=public",
],
),
)
def test_make_connection_uri(username_password_port, expected):
username, password, port = username_password_port
assert (
make_connection_uri(
PostgresDBConfig(
host="localhost",
name="dbname",
username=username,
password=password,
db_schema="public",
port=port,
)
)
== expected
)


def test_get_connection_parameters(monkeypatch: pytest.MonkeyPatch):
monkeypatch.delenv("DB_SSL_MODE")
db_config = get_db_config()
Expand Down
2 changes: 1 addition & 1 deletion app/tests/src/db/test_migrations.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging # noqa: B1

import alembic.command as command
import pytest
from alembic import command
from alembic.script import ScriptDirectory
from alembic.script.revision import MultipleHeads
from alembic.util.exc import CommandError
Expand Down