model_definitions_match_ddl should delete the generated revision

[schireson]

model_definitions_match_ddl should delete the generated revision

[DanCardin]

Odd that I dont see this behavior in the pytest-alembic tests themselves / our own histories.

We do have some specific code to preempt the revision from being generated, so i have a series of questions:

• Are the autogenerations left behind empty?
• What version of alembic are you using?
• do you have a process_revision_directives set in your env.py?
• Have you configured a specific pytest-alembic config through one of the config mechanisms, and if so how does that look?

One thing I know we do across the board here (although not in the pytest-alembic tests...) is not generate revisions when they're empty, but like i said we have code in place which ought to prevent it from being generated whether it was successful or not that appears to be working in all the configurations we've tested on.

[romainrichard]

Thanks for the quick reply! Let me try to get you the information you need.

• Are the autogenerations left behind empty?

They're not empty files but there's no code in the upgrade/downgrade functions. Here's an example:

$ cat alembic/versions/b32c6426366d_test_revision.py
"""test revision.

Revision ID: b32c6426366d
Revises: 9197d6dab9fd
Create Date: 2022-05-11 22:37:36.646186

"""
import sqlalchemy as sa

from alembic import op

# revision identifiers, used by Alembic.
# pragma: allowlist nextline secret
revision = 'b32c6426366d'
# pragma: allowlist nextline secret
down_revision = '9197d6dab9fd'
branch_labels = None
depends_on = None


def upgrade() -> None:
    """Upgrade schema migrations."""
    pass


def downgrade() -> None:
    """Downgrade schema migrations."""
    pass

• What version of alembic are you using?

1.7.7 (the latest one currently)

• Do you have a process_revision_directives set in your env.py?

No

• Have you configured a specific pytest-alembic config through one of the config mechanisms, and if so how does that look?

No. Can't share the files but I can list the changes that I made to try out pytest-alembic:

1. Installed pytest-alembic
2. Added --test-alembic to the pytest invocation
3. Updated alembic/env.py based on https://pytest-alembic.readthedocs.io/en/latest/setup.html otherwise a lot of errors would show up, but had to do things a bit differently otherwise errors kept showing up (some from pytest but other tools complained too, like mypy). So currently it looks basically like this:

async def run_migrations_online() -> None:
    connectable = context.config.attributes.get("connection", None)
    if connectable is None:
        connectable = AsyncEngine(
            engine_from_config(
                config.get_section(config.config_ini_section),
                prefix="sqlalchemy.",
                poolclass=pool.NullPool,
            )
        )

        # If this isn't in the if block, it'll raise an error saying that a bool doesn't have a connect attribute.
        async with connectable.connect() as connection:
            await connection.run_sync(do_run_migrations)

[DanCardin]

        # If this isn't in the if block, it'll raise an error saying that a bool doesn't have a connect attribute.
        async with connectable.connect() as connection:
            await connection.run_sync(do_run_migrations)

fwiw pytest-alembic is automatically supplying the connectable to you under the hood through the config attributes collection, based on how you set the alembic_engine/alembic_config fixtures. So i wouldn't expect this conditional to get hit during tests. Which makes this block seem somewhat suspicious; without the call to run_sync, it's not clear to me what'd be going on.

See https://pytest-alembic.readthedocs.io/en/latest/asyncio.html, if you haven't already.

Also we have a couple of async examples which might be helpful, depending on the way your engines are being set up in your actual application:
https://github.com/schireson/pytest-alembic/tree/main/examples/test_async_sqlalchemy
https://github.com/schireson/pytest-alembic/tree/main/examples/test_async_sqlalchemy_native

What does your alembic_engine fixture look like? that might be another guess, if you're not setting that and it's defaulting to a synchronous sqlite one.

[romainrichard]

Thanks, I went through those and tried a few different combinations. I always get to one of those 2 outcomes:

1. The test revision file is left behind
2. The code will raise an exception (most often because of AttributeError: 'bool' object has no attribute 'connect' but sometimes I'd get something else if I defined alembic_engine)

I've tried multiple variations of alembic/env.py based on the examples you mentioned. But none did the trick.

I did not override the alembic_engine/alembic_config previously so I also gave that a shot. And there was definitely something off there at first. I figured out that I had to define pytest_alembic_tests_folder since my test folder isn't tests/. Once I did that it the fixtures started working (as in, pytest was actually using them).
I tried create_async_engine and create_postgres_fixture for the alembic_engine fixture but none fully worked.

I can try doing a more detailed write-up, meaning showing the different configs I tried and their respective results. I did try quite a few so if there are any you'd like to get more info on let me know which ones.

[DanCardin]

I'm honestly baffled by the fact of it leaving behind a file. I'm decently certain it must be a side-effect of how the env.py is defined (even if it's just only code paths being hit by pytest-alembic, and your literal migrations work fine). If it's possible to construct a minimally failing example set of files, that'd greatly help my ability to help you.

But yea, if your preexisting env.py is set up with async engines, then you'll definitely need to define an alembic_engine (the default one would be sync) which yields an async engine.

Fwiw, that bool error implies connectable is a bool, which implies that alembic_engine is returning a bool.

[romainrichard]

It definitely looks like I need to do more investigation around our env.py and other fixtures that should be defined. I'll come back to this later, so in the meantime I'll close this issue.
Thanks for you help.

[kamenomagic]

I am also seeing this issue; I'm not sure why I was not experiencing it previously, but I moved some stuff around and it now generates that empty revision as shown above. Let me know if I can help/take a look at something.

[kamenomagic]

Oh I just figured something out. If I reverence the env.py file instead of put the code directly, it leaves the file behind. If I put the code directly in the env.py (as is normal) it doesn't leave the file behind.

[kamenomagic]

I'm guessing the run_migrations() methods are being called on import when I reference.

This may or may not be related to this issue because of that. But maybe worth commenting about so I'll leave these here.