788 alembic

[SofiaSazonova]

Feature or Bugfix

• Bugfix
• Refactoring

Detail

1. Alembic migrations are now synchronised with the current code-state
2. make generate-migrations command is added. It can be applied right after the docker container with psql is started
3. Readme about migrations

Relates

data.all in local environment starts in a broken db state #788

Security

N/A

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[dlpzx]

Some changes needed

[dlpzx]

Left some final touches and comments on the actual migration script

[noah-paige]

I think this PR is close - the one last thing I would add is a better way to handle upgrades for local deployments of dataall (i.e. when envname='dkrcompose').

I tested the following change out in dataall/backend/dataall/base/db/connection.py (L124 - 130) and I think it would fix some of the issues I had faced in the past while not impacting local data.all deployments - where I was changing hostname manually to get a alembic migration to run properly from my local to the db container:

        db_params = {
            'host': 'db' if envname == 'dkrcompose' and os.path.exists("/.dockerenv") else 'localhost',
            'db': 'dataall',
            'user': 'postgres',
            'pwd': 'docker',
            'schema': envname,
        }

I think the final change(s) I recommend is we add the above change and a Note in the "Managing migrations" Section of the README.md that mentions you can apply the same upgrades against your db schema that is used in local data.all deployments by just exporting envname=dkrcompose and using the same commands (i.e. alembic -c backend/alembic.ini upgrade head ...)

[dlpzx]

From a code perspective I think this PR is good to go. I will do a final test by deploying the PR to an AWS deployment to verify that the migrations run successfully on pre-existing AWS deployments.

[dlpzx]

Tested in AWS:

• CICD pipeline succeeds. But, I checked the CodeBuild migration logs and the migration script is called: INFO [alembic.runtime.migration] Running upgrade f6cd4ba7dd8d -> 6c9a8afee4e4, _describe_changes_shortly Maybe we should rename in the alembic history and avoid the extra "_"
• Pre-existing Datasets, MLStudio and Environments are listed as usual after the migration

[noah-paige]

I think this PR looks good to merge! Thanks @SofiaSazonova for dealing with a bunch of iterations on this one 😄