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

[LLSC-53] Replace alembic migrate on startup with SQLAchemy create table #15

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mslwang
Copy link
Collaborator

@mslwang mslwang commented Dec 8, 2024

Notion ticket link

Remove Alembic migration on startup

The alembic.ini and env.py config is taking control of the logging system when running run_migrations using alembic.command

def run_migrations():
, meaning errors that should normally be displayed are not being displayed.

This is a bigger priority for productivity to maintain over worrying about up-to-date database migrations, so we need to remove the current system of making sure databases are up to date.

We will replace this with SQLAlchemy create table, which creates tables based on the models, not the migrations. We should conduct a further investigation of whether migration-based table management will be necessary.

Implementation description

Steps to test

  1. Clear docker volume for Postgres database
  2. Start up server pdm run dev
  3. Check if API can be hit and all tables are correct

What should reviewers focus on?

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

@mslwang
Copy link
Collaborator Author

mslwang commented Jan 1, 2025

As mentioned on slack, this is currently flawed because lookup tables aren't populated with this approach (eg. Roles table needs the database migration)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant