migrate command not passing the autogenerate ops

[andre-qumulo]

I am migrating from flask-migrate 2.5.3 to the latest. I have updated my env.py file to:

def run_migrations_online() -> None:
    """Run migrations in 'online' mode.

    In this scenario we need to create an Engine
    and associate a connection with the context.

    """

    # this callback is used to prevent an auto-migration from being generated
    # when there are no changes to the schema
    # reference: http://alembic.zzzcomputing.com/en/latest/cookbook.html
    def process_revision_directives(
        context: MigrationContext,  # noqa: ARG001
        revision: _GetRevArg,  # noqa: ARG001
        directives: List[MigrationScript],
    ) -> None:
        if getattr(config.cmd_opts, 'autogenerate', False):
            script = directives[0]
            if script.upgrade_ops is not None and script.upgrade_ops.is_empty():
                directives[:] = []
                logger.info('No changes in schema detected.')

    conf_args = current_app.extensions['migrate'].configure_args
    if conf_args.get('process_revision_directives') is None:
        conf_args['process_revision_directives'] = process_revision_directives

    connectable = get_engine()

    cm = retry.on_exception(should_retry_connect, delay_sec=5)(connectable.connect)()
    with cm as connection:
        context.configure(
            connection=connection,
            target_metadata=get_metadata(),
            user_module_prefix='models.',
            **conf_args,
        )

        with context.begin_transaction():
            context.run_migrations()

What I am finding is that when I run a function that:

1. Creates a new DB
2. Runs all existing migrations on it
3. runs the upgrade command

It is no longer passing the autogenerate ops to the alembic command. That is getattr(config.cmd_opts, 'autogenerate', False) is evaluating to False.

I can find no other references to autogenerate in the code base so I am wondering if it needs to be passed in somewhere now?

[andre-qumulo]

I think I've figured out the answer. It might be just my code but the value of config.cmd_opts has autogenerate=True when the execution enters these lines:

with context.begin_transaction():
            context.run_migrations()

but by the time it gets to the defined process_revision_directives functions in the file, config.cmd_opts does not have autogenerate at all.
The revision context, however, does. So I changed if getattr(config.cmd_opts, 'autogenerate', False): to if getattr(context.config.cmd_opts, 'autogenerate', False): and it works.

I do not know if this is a bug in https://github.com/miguelgrinberg/Flask-Migrate/blob/main/tests/custom_template/env.py#L67 but I don't see any specific tests of this path in https://github.com/miguelgrinberg/Flask-Migrate/blob/main/tests/test_migrate.py

[andre-qumulo]

I changed

    def process_revision_directives(
        context: MigrationContext,  # noqa: ARG001
        revision: _GetRevArg,  # noqa: ARG001
        directives: List[MigrationScript],
    ) -> None:
        if getattr(config.cmd_opts, 'autogenerate', False):
            script = directives[0]
            if script.upgrade_ops is not None and script.upgrade_ops.is_empty():
                directives[:] = []
                logger.info('No changes in schema detected.')

to

    def process_revision_directives(
        context: MigrationContext,
        revision: _GetRevArg,  # noqa: ARG001
        directives: List[MigrationScript],
    ) -> None:
        if context.config is not None and getattr(
            context.config.cmd_opts, 'autogenerate', False
        ):
            script = directives[0]
            if script.upgrade_ops is not None and script.upgrade_ops.is_empty():
                directives[:] = []
                logger.info('No changes in schema detected.')

I'm happy to share the entire env.py file if you'd like.

Our test case calls this function:

def migrate_database(fs: FileSystem, description: str) -> None:
    logging.info(f'pwd is {os.getcwd()}')
    migration_dir = f'{os.path.dirname(os.path.realpath(__file__))}/{MIGRATION_DIR}'
    fs.mkdir(migration_dir, ignore_existing=True)
    _head_path, head_num = get_head_migration(fs, migration_dir)
    next_migration_num = head_num + 1

    migration_dir = fs.translate(migration_dir)
    with postgres_server(fs) as postgres_client, create_app(
        {'SQLALCHEMY_DATABASE_URI': postgres_client.get_url()}
    ).app_context():
        upgrade(migration_dir)
        migrate(migration_dir, message=f'{next_migration_num} {description}')

create_app does the following:

def create_app(
    test_config: Optional[Dict[str, Any]] = None,
) -> Flask:
    set_up_logging(__name__)

    app = Flask(__name__, instance_relative_config=True)
    _ = FlaskMiddleware(app)
    configure_gunicorn_logging(app)

    if test_config is None:
        app.config['SQLALCHEMY_DATABASE_URI'] = os.environ['SQLALCHEMY_DATABASE_URI']
        app.config['SQLALCHEMY_ECHO'] = False
    else:
        app.config.from_mapping(test_config)

    # we don't use SQLAlchemy events and this particular event is deprecated in the future
    app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False

    db.init_app(app)
    migrate.init_app(app, db)
    with contextlib.suppress(OSError):
        os.makedirs(app.instance_path)

    app.register_blueprint(api) # this is registering our flask routes

    return app

Is it because we're passing instance_relative_config=True to Flask? I hadn't noticed that before

[miguelgrinberg]

I'm having trouble understanding what the problem is here. You are looking at this autogenerate option that you think should be True but you find to be False, I do understand that. But is there any problem that this is causing?

The default for autogenerate is False, and only when you use the migrate command it is set to True. This is expected. The code that you are changing that looks at this configuration option isn't part of the core migration logic, it just tries to detect if an autogenerated migration is empty.

[andre-qumulo]

I'm saying that I'm unsure if the code that looks at the config is getting the wrong config for the migration.
if getattr(config.cmd_opts, 'autogenerate', False): is getting the global config but not the config from the migration context it's running against.

I think that's a bug but wanted to get your opinion as I copied it from this github repo.

[miguelgrinberg]

Can you please tell me if there is anything that isn't working correctly? Are your migrations not working? I need to know what is the problem, you have not told me that yet.

As I said above, the code that you are looking at that uses the autogenerate variable does not perform migrations. You can delete that code block and everything will continue to work just fine, save for a minor convenience feature.

[andre-qumulo]

My code is working but we had a test case that relied on the error message from that code block.
I wanted to let you know how I fixed it in case other people have a similar problem.