Modification on slices table will fail if following the history of migration

[axeisghost]

We tried to add one more properties of Slices, so it involved the addition of columns in the table of slices. We modified the models.py, views.py and added new version file. However, if run the caravel db grade from the initialization, the migration will fail because in the version file
c3a8f8611885_materializing_permission.py, the migration process will use models.Slice in models.py.

    session = db.Session(bind=bind)

    for slc in session.query(models.Slice).all():
        if slc.datasource:
            slc.perm = slc.datasource.perm
            session.merge(slc)
            session.commit()
    db.session.close()

At this time, the database has not been fully upgraded and some columns in slice that has not been added, so using models.Slice will cause runtime error. This issue has not been found before mainly because there is no commit modifying the table of slices. This version file will affect all the modification on slice tables after this version.

Basically, this migration add the perm column for slices and copy the content perm column of corresponding datasource. In my opinion, it is not necessary to do this modification in migration. The perm column can be leaved as blank and whenever the query result of this column is empty, the perm of datasource will be queryed and used as the perm of certain slice.

[mistercrunch]

We need perm in Slice for good reasons, it helps generating custom list of Slices and Dashboards on the fly based on security settings.
https://github.com/airbnb/caravel/blob/master/caravel/views.py#L91

The query is already pretty complicated with perm in Slice and would probably need to be a two phase query if it wasn't in there which is bad.

I don't get what the problem is with this migration script.

[axeisghost]

Thanks. I understand that perm is necessary. The migration of the version will use models.Slice class to do the query for the perm in the migration process but at that time the database is not fully migrated yet. If any migration after this one add a new column for slices, the models.Slice class will declare the new column too. Before reaching the new column migration, using models.Slice will cause the SQLAlchemy fail to bind that new column and pop up error "no such column"