Is invoking SQLAlchemy's declarative_base()—bypassing Flask-SQLAlchemy's models—a sanctioned workflow?

[john-bodley]

In Apache Superset we use Flask-Migrate (in conjunction with Flask-SQLAlchemy) to handle our Alembic migrations. For a specific migration which augments the contents of the data (as opposed to the schema) we tend to redefine our models (with only the subset of impacted columns) via SQLAlchemy (as opposed to Flask-SQLAlchemy) and use Alembic for interfacing with the bind/connection:

from alembic import op
from sqlalchemy import Column, Integer, String
from sqlalchemy.ext.declarative import declarative_base

from superset import db

Base = declarative_base()


class Dashboard(Base):
    __tablename__ = "dashboards"

    id = Column(Integer, primary_key=True)
    title = Column(String(length=256))
    

def upgrade():
    session = db.Session(bind=op.get_bind())

    for dashboard in session.query(Dashboard).all():
        dashboard.title = dashboard.title.upper()

    session.commit()

To the best of my knowledge, the historic reason for using SQLALchemy's declarative_base() method (as opposed to db.Model provided by Flask-SQLAlchemy) is:

1. It provides a "frozen" (point-in-time) representation of the model—as opposed to the model in the application.
2. It ensures that various mixins which are associated with the model—like Flask-AppBuilder's AuditMixin are not impacted by upgrade, i.e., we want to ensure that the changed_on and changed_by fields are not updated when the records are persisted to the database.

I'm currently working on a refactor (apache/superset#26172) to try to ensure Flask-Migrate leverages the Flask-SQLAlchemy session (db.session)—shedding over 1,000 lines—where the previous faux migration would be redefined as:

from superset import db


class Dashboard(db.Model):
    __tablename__ = "dashboards"
    __table_args__ = {"extend_existing": True}

    id = db.Column(Integer, primary_key=True)
    title = db.Column(db.String(length=256))
    

def upgrade():
    for dashboard in db.session.query(Dashboard).all():
        dashboard.title = dashboard.title.upper()

    db.session.commit()

The extend_existing=True argument is provided to resolve,

sqlalchemy.exc.InvalidRequestError: Table 'dashboards' is already defined for this MetaData instance.  Specify 'extend_existing=True' to redefine options and columns on an existing Table object.

however this merely extends (as opposed to replacing) the Dashboard model defined within the application. This can be problematic when after years of development the schema of the current model has likely evolved considerably to that of the model perceived by a legacy migration and there's no way to exclude columns—such as those provided by Flask-AppBuilder's AuditMixin.

I've struggled to find how to resolve this—either via numerous Google searches or ChatGPT—and was wondering what preferred Flask-Migrate/Flask-SQLAlchemy approach is? Is the declarative_base route the only viable—albeit somewhat non-Flask-SQLAlchemy-esque—approach if one requires fine level controls at the model level?

[miguelgrinberg]

I'm not sure who decides what is sanctioned and what is not. I don't think it's me. ;-)

Now if you are asking me if I like this solution, then no, I do not like it. You are working with two declarative bases, the one from Flask-SQLAlchemy and one more that you define yourself.

The part that I do agree with you with is that sometimes it is convenient to have access to models in migration scripts, and in that case the only way to make a migration script work well is to include a copy of the model(s) in question in the migration script itself. Looks like you already figured out that the important rule is to never use your application's models in your migrations scripts.

I think your issue is that when you do from superset import db you are indirectly importing your regular models, the ones in your application. This is actually breaking the rule above. You definitely do not want your regular models to exist in the context of your migration scripts. The only models that you want in such a situation are the ones that are explicitly defined in the script. Any and all models that you need to use in your script should be copied.

So I'd say these are the recommendations:

• If you can get away without using models in your migration scripts, that is much easier.
• If you must use models, then make sure you do not import your regular models, and copy any models that you need into the script.
• You have to be careful with indirect imports. Running from superset import db should not pull your models for the migration script to work well with the copied models. That will ensure there are no duplicates, so you will not need to use strange/obscure options to avoid strange/obscure SQLAlchemy errors.

Hope this helps!

[john-bodley]

Thanks @miguelgrinberg for the timely and comprehensive response.

Superset definitely has some atypical import patterns which may inadvertently be importing regular models, however I'm still unable to decouple the metadata, even via the following simple toy example (derived from the official documentation which decouples the definitions of the model, extensions, and application to prevent indirect imports):

# extensions.py
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate

db = SQLAlchemy()
migrate = Migrate()

# models.py
from extensions import db


class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(length=128))

# app.py
from flask import Flask

from extensions import db, migrate


def create_app() -> Flask:
    app = Flask(__name__)
    app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///app.db"

    db.init_app(app)
    migrate.init_app(app, db)

    # Importing the models ensures they are known to Flask-Migrate.
    from models import User

    return app

# migrations/versions/c48f49c7d4f4.py
from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision = "c48f49c7d4f4"
down_revision = None
branch_labels = None
depends_on = None


def upgrade() -> None:
    op.create_table(
        "user",
        sa.Column("id", sa.Integer(), nullable=False),
        sa.Column("name", sa.String(length=128), nullable=True),
        sa.PrimaryKeyConstraint("id"),
    )


def downgrade() -> None:
    op.drop_table("user")

# migrations/versions/aeb4e6fc7fb.py
from extensions import db

# revision identifiers, used by Alembic.
revision = "aeb4e6fc7fb"
down_revision = "c48f49c7d4f4"
branch_labels = None
depends_on = None


class User(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(length=128))


def upgrade() -> None:
    for user in db.session.query(User).all():
        user.name = user.name.upper()

    db.session.commit()


def downgrade() -> None:
    for user in db.session.query(User).all():
        user.name = user.name.lower()

    db.session.commit()

which results in the following error:

> flask db upgrade
...
sqlalchemy.exc.InvalidRequestError: Table 'user' is already defined for this MetaData instance.  Specify 'extend_existing=True' to redefine options and columns on an existing Table object.

A global GitHub search for examples of Flask-Migrate migrations with models redefined within a migration with db.Model as the base (as opposed to defining another SQLAlchemy declarative base) seems to illustrate the use of the extend_existing=True option, however I perceive you sense this should be avoided per your "so you will not need to use strange/obscure options to avoid strange/obscure SQLAlchemy errors" comment.

The TL;DR is I'm a tad unsure of how best to proceed. It seems like one could clear all the table metadata within the confines of a migration, per SQLAlchemy's MetaData.clear() method (likely invoked in migrations/env.py), though this seems somewhat atypical. Any thoughts @miguelgrinberg?

[miguelgrinberg]

Yes, I see now that what I suggested is not really possible. The project in which I've done this was implemented with SQLAlchemy and Alembic directly, not with Flask and Flask-Migrate. That gives you more control over what gets imported. The flask command imports your Flask app, and to create this object pretty much everything must be imported.

Unfortunately I do not have any great ideas. I think every solution is going to be hacky. I think the only reasonable option is to avoid using models in migration scripts, which is actually what the vast majority of people do, what 99.9% of projects I have been involved with do, and what the automatic migration generator does as well.

[john-bodley]

Thanks again @miguelgrinberg for the response. Apache Superset tends to (overly) store data as JSON blobs with fairly complex mappings and thus mutating the data is often only viable—via the models—in Python.