alembic check fails when not generating empty migration scripts

[BayerSe]

Hi,

I'm trying to re-add skipping of empty migrations to our env.py file. For that, I am currently testing the snippet proposed here,

def process_revision_directives(
        context: MigrationContext,
        revision: tuple[str, str],
        directives: list[MigrationScript],
    ):
        assert config.cmd_opts is not None
        if config.cmd_opts.autogenerate:
            script = directives[0]
            assert script.upgrade_ops is not None
            if script.upgrade_ops.is_empty():
                directives[:] = []

with that, alembic revision --autogenerate -m ... works perfectly, no migrations are generated.

However, alembic check raises: AttributeError: 'Namespace' object has no attribute 'autogenerate'.

Here's the stacktrace

INFO  [alembic.runtime.migration] Context impl OracleImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
Traceback (most recent call last):
  File "/home/user/PycharmProjects/project/.venv/bin/alembic", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 630, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 624, in main
    self.run_cmd(cfg, options)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 601, in run_cmd
    fn(
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/command.py", line 289, in check
    script_directory.run_env()
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/script/base.py", line 578, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 93, in load_python_file
    module = load_module_py(module_id, path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 109, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/user/PycharmProjects/project/scheduler/alembic/env.py", line 188, in <module>
    run_migrations_online()
  File "/home/user/PycharmProjects/project/scheduler/alembic/env.py", line 182, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/runtime/environment.py", line 937, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/runtime/migration.py", line 612, in run_migrations
    for step in self._migrations_fn(heads, self):
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/command.py", line 278, in retrieve_migrations
    revision_context.run_autogenerate(rev, context)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/autogenerate/api.py", line 560, in run_autogenerate
    self._run_environment(rev, migration_context, True)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/autogenerate/api.py", line 618, in _run_environment
    hook(migration_context, rev, self.generated_revisions)
  File "/home/user/PycharmProjects/project/scheduler/alembic/env.py", line 147, in process_revision_directives
    if config.cmd_opts.autogenerate:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Namespace' object has no attribute 'autogenerate'

In the past, we did that with a rewriter,

writer = rewriter.Rewriter()
@writer.rewrites(ops.MigrationScript)
def suppress_empty_migration_script(
    context: MigrationContext, revision: tuple[str], op: ops.MigrationScript
) -> ops.MigrationScript:
    """If there is nothing to do, do NOT create an (empty) migration script, see
    https://alembic.sqlalchemy.org/en/latest/cookbook.html#cookbook-no-empty-migrations.
    """
    if op.upgrade_ops.is_empty():
        op = []
    return op

but that also fails nowadays, with IndexError: list index out of range.

Here's the stacktrace

INFO  [alembic.runtime.migration] Context impl OracleImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
Traceback (most recent call last):
  File "/home/user/PycharmProjects/project/.venv/bin/alembic", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 630, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 624, in main
    self.run_cmd(cfg, options)
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/config.py", line 601, in run_cmd
    fn(
  File "/home/user/PycharmProjects/project/.venv/lib/python3.11/site-packages/alembic/command.py", line 293, in check
    migration_script = revision_context.generated_revisions[-1]
                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^
IndexError: list index out of range

Is this a bug in alembic, or is this a bug in the proposed snippet?

Thanks
Sebastian

[CaselIT]

Hi,

For that, I am currently testing the snippet proposed here,

the issue seems to be in the snippet.
try using if getattr(config.cmd_opts, 'autogenerate', False):.

If that work we can update the snippet

[BayerSe]

Works! Thanks. I can create a PR if you like.

Can you maybe give me a hint how I would chain process_revision_directives with rewriters?

writer.chain(process_revision_directives)

works, but this somewhat misuses the signatures, if I see this correctly.

[CaselIT]

I'm not sure if that use case is supported, but I can see that if works ok.

@zzzeek should this be officially supported updating the docs/types?

[zzzeek]

ive just read the source code for chain() and it's a bit of a disaster. because if i do this:

w = writer1.chain(writer2).chain(writer3)

writer2 is lost. it's doing the chaining wrong. my high school comp sci teacher would be unimpressed

it works for your use case right now because it only handles exactly two writers. but if we fix it to correctly implement chaining then it would not work for your case in the general sense.

we can fix both issues at once by 1. fixing the chaining so that if self._chain is non-None we traverse the list through to the end for the linked list append, and 2. looking at the type of object sent and if it's just a callable, wrapping it in a helper object CallableRewriter that uses that callable and also supports the rest of the Rewriter protocol including self._chain.

[CaselIT]

ok, that sounds like an issue.

Regarding the implementation, indeed chain is not great, since it already creates a copy of the class

[CaselIT]

I can create a PR if you like.

that would be great

[BayerSe]

Done: #1338