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

Ticket 37 - move init.sql from sbl-project repo to user-fi-management #43

Merged
merged 22 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 14 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
67 changes: 49 additions & 18 deletions db_revisions/env.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import os
from dotenv import load_dotenv

from logging.config import fileConfig

from sqlalchemy import engine_from_config
from sqlalchemy import pool

from alembic import context
from entities import models
from src.entities.models import Base
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't need to have src in the path, should follow what it was previously, and just be entities..., you want to import Base directly, do entities.models


# this is the Alembic Config object, which provides
# access to the values within the .ini file in use.
Expand All @@ -18,17 +15,6 @@
if config.config_file_name is not None:
fileConfig(config.config_file_name)

# add your model's MetaData object here
# for 'autogenerate' support
# from myapp import mymodel
# target_metadata = mymodel.Base.metadata
target_metadata = models.Base.metadata

# other values from the config, defined by the needs of env.py,
# can be acquired:
# my_important_option = config.get_main_option("my_important_option")
# ... etc.

# this specific to SBL configuration

ENV = os.getenv("ENV", "LOCAL")
Expand All @@ -42,11 +28,31 @@
INST_DB_PWD = os.environ.get("INST_DB_PWD")
INST_DB_HOST = os.environ.get("INST_DB_HOST")
INST_DB_NAME = os.environ.get("INST_DB_NAME")
INST_DB_SCHEMA = os.environ.get("INST_DB_SCHEMA")
INST_CONN = f"postgresql://{INST_DB_USER}:{INST_DB_PWD}@{INST_DB_HOST}/{INST_DB_NAME}"
config.set_main_option("sqlalchemy.url", INST_CONN)

# end specific SBL configuration

# add your model's MetaData object here
# for 'autogenerate' support
# from myapp import mymodel

target_metadata = Base.metadata
target_metadata.schema = INST_DB_SCHEMA

# other values from the config, defined by the needs of env.py,
# can be acquired:
# my_important_option = config.get_main_option("my_important_option")
# ... etc.


def include_object(object, name, type_, reflected, compare_to):
if type_ == "table" and name == "alembic_version":
Copy link
Collaborator

Choose a reason for hiding this comment

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

we definitely need to have the alembic_version table to be included, this is how alembic tracks what has been ran, and what hasn't.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also, we'll need to update the configuration to include version_table_schema since on local machine it'll be the default public, but in other environments, it'll be different. look into https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.environment.EnvironmentContext.configure.params.version_table_schema

return False
else:
return True


def run_migrations_offline() -> None:
"""Run migrations in 'offline' mode. This generates the SQL script without executing on the database.
Expand Down Expand Up @@ -79,17 +85,42 @@ def run_migrations_online() -> None:
and associate a connection with the context.

"""
connectable = engine_from_config(

connectable = context.config.attributes.get("connection", None)

if connectable is None:
connectable = engine_from_config(
context.config.get_section(context.config.config_ini_section),
prefix="sqlalchemy.",
poolclass=pool.NullPool,
)

with connectable.connect() as connection:
context.configure(
connection=connection,
target_metadata=target_metadata,
version_table_schema=target_metadata.schema,
include_object=include_object,
)

with context.begin_transaction():
context.run_migrations()
"""connectable = engine_from_config(
config.get_section(config.config_ini_section, {}),
prefix="sqlalchemy.",
poolclass=pool.NullPool,
)

with connectable.connect() as connection:
context.configure(connection=connection, target_metadata=target_metadata)
context.configure(
connection=connection,
target_metadata=target_metadata,
version_table_schema=target_metadata.schema,
include_object=include_object,
)

with context.begin_transaction():
context.run_migrations()
context.run_migrations()"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this section a docstring? looks like it's the same code as above?



if context.is_offline_mode():
Expand Down
2 changes: 1 addition & 1 deletion db_revisions/script.py.mako
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ def upgrade() -> None:


def downgrade() -> None:
${downgrades if downgrades else "pass"}
${downgrades if downgrades else "pass"}
69 changes: 69 additions & 0 deletions db_revisions/versions/26556a57a06f_create_a_baseline_migrations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
"""Create a baseline migrations

Revision ID: 26556a57a06f
Revises:
Create Date: 2023-10-23 04:04:07.722956

"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision: str = "26556a57a06f"
down_revision: Union[str, None] = None
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"denied_domains",
sa.Column("domain", sa.String(), nullable=False),
sa.Column("event_time", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
sa.PrimaryKeyConstraint("domain"),
)
op.create_index(op.f("ix_denied_domains_domain"), "denied_domains", ["domain"], unique=False)
op.create_table(
"financial_institutions",
sa.Column("lei", sa.String(), nullable=False),
sa.Column("name", sa.String(), nullable=False),
sa.Column("event_time", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
sa.PrimaryKeyConstraint("lei"),
)
op.create_index(op.f("ix_financial_institutions_lei"), "financial_institutions", ["lei"], unique=True)
op.create_index(op.f("ix_financial_institutions_name"), "financial_institutions", ["name"], unique=False)
op.create_table(
"financial_institution_domains",
sa.Column("domain", sa.String(), nullable=False),
sa.Column("lei", sa.String(), nullable=False),
sa.Column("event_time", sa.DateTime(), server_default=sa.text("now()"), nullable=False),
sa.ForeignKeyConstraint(
["lei"],
["public.financial_institutions.lei"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

is the schema name needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lchen-2101 removed in latest commit.

),
sa.PrimaryKeyConstraint("domain", "lei"),
)
op.create_index(
op.f("ix_financial_institution_domains_domain"), "financial_institution_domains", ["domain"], unique=False
)
op.create_index(
op.f("ix_financial_institution_domains_lei"), "financial_institution_domains", ["lei"], unique=False
)
# ### end Alembic commands ###


def downgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this auto-generated comment?

op.drop_index(op.f("ix_financial_institution_domains_lei"), table_name="financial_institution_domains")
op.drop_index(op.f("ix_financial_institution_domains_domain"), table_name="financial_institution_domains")
op.drop_table("financial_institution_domains")
op.drop_index(op.f("ix_financial_institutions_name"), table_name="financial_institutions")
op.drop_index(op.f("ix_financial_institutions_lei"), table_name="financial_institutions")
op.drop_table("financial_institutions")
op.drop_index(op.f("ix_denied_domains_domain"), table_name="denied_domains")
op.drop_table("denied_domains")
Copy link
Contributor

Choose a reason for hiding this comment

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

I may have missed comment's reply on drop_index. Do we need drop_index even though we're going to drop the table anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it was removed but it was generated when I tried to fix the previous issues. Removing them again.

# ### end Alembic commands ###
2 changes: 0 additions & 2 deletions db_revisions/versions/README

This file was deleted.

Loading
Loading