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

Current repo doesn't support hosting a local db #194

Open
widal001 opened this issue Nov 23, 2020 · 1 comment
Open

Current repo doesn't support hosting a local db #194

widal001 opened this issue Nov 23, 2020 · 1 comment
Assignees

Comments

@widal001
Copy link
Contributor

Summary of issue

Currently, attempting to run scripts/init_localdb.sh (or any other script that calls it) will fail with an error message that looks similar to the output below, which prevents collaborators from hosting a local version of the database:

sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column "program_name" does not exist
LINE 2:                WHERE program_name = 'Mayoral Fellowship'

Background on this issue

Current approach

Currently scripts/init_localdb.sh works by creating a new docker container called hippo_localdb and then attempts to set it up with the following commands:

if ! run_db_cmd localdb -c "SELECT 1" >/dev/null; then
    echo "Creating database localdb..."
    run_db_cmd -c "CREATE DATABASE localdb"
    source env/bin/activate
    python migrate.py db upgrade
    echo "Created database localdb."
    cat common/local_base_data.sql | run_db_cmd localdb
    python init_db_local.py
    echo "Added base data."
else
    echo "Database localdb already found, skipping init"
fi

The gist of these lines is that it activates the virtual environment, then runs python migrate.py db upgrade to update the db schema using the migration scripts available in the current directory, and ultimately populates the newly upgraded db with the script init_db_local.py

Problems with this approach

  • Alembic (which is what executes the migration script) is not currently set up to track migrations for multiple databases, so any changes or inconsistencies between the current state of the prod db (which is what the migration scripts are currently modifying) and the local db will cause python migrate.py db upgrade to break
  • Even if the the prod and local databases remained consistent with one another, this approach expects maintenance of a separate local_base_data.sql to populate the database with test data

Solutions

Temporary fix

The temporary fix is to simply resolve issues encountered in the db upgrade by modifying the lines in the migration script that produce these errors

Steps

  1. Run scripts/init_localdb.sh and follow the stack trace error(s) that occur when using the migration scripts to upgrade the local db
  2. Go to the line in the problem migration script and comment out or modify the line in the migration script that is causing the error
  3. Repeat steps 1 and 2 until the upgrade successfully passes
  4. Discard temporary changes made to the migration scripts to avoid committing them

Long-term fix A

One long term strategy is to maintain separate sets of migration scripts for the local and prod databases and modify the scripts/init_localdb.sh to reference the appropriate set of migration scripts when instantiating the db

Steps

  1. Take advantage of the flask-migrate option to support multiple databases
  2. Refactor the migrations folder to track separate migration scripts for the prod, staging, and local database
  3. Modify scripts/init_localdb.sh to use the migrations specific to the local db

Long-term fix B

Another long-term strategy is to abandon the use of migration scripts altogether and simply blow away and rebuild the local database directly from the model, similar to what we do within testing.

Steps

  1. Modify app.py or create a new python script that initializes a local db each time python run.py is called
  2. The script would simply create a fresh db using a strategy similar to the one employed in conftest.py by calling db.create_all()
  3. This script would also populate the local database using populate_db.py like conftest.py does

Recommendation

To achieve the primary goal of being able to host and connect to a local database, I would recommend pursuing option B. Option B has the added benefit of being able to instantly create a fresh db instance that is unique to the active branch without having to maintain separate db migrations for each branch--which would be difficult to do even with the multiple db option enabled for flask-migrate

Option A is worth considering to maintain separate migration scripts for the staging and prod databases, but this option should be pursued only after we've successfully implemented or exhausted option B

@afialydia
Copy link
Contributor

Hi Billy! Great solutions!

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

No branches or pull requests

2 participants