`MigrationContext.previous_revision()` returns wrong revision with nonlinear history

[olmokramer]

I have the following revisions:

"""
Revision ID: 7f5b06f81aef
Revises: effc355fddff
Create Date: 2020-07-01 16:44:05.244310
"""
from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision = '7f5b06f81aef'
down_revision = 'effc355fddff'
branch_labels = None
depends_on = None

def upgrade():
    pass # upgrade code redacted

def downgrade():
    pass # downgrade code redacted

"""
Revision ID: 79ee02b5e895
Revises: 10483a8dd3c8, effc355fddff
Create Date: 2020-06-23 11:04:51.901600
"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = '79ee02b5e895'
down_revision = ('10483a8dd3c8', 'effc355fddff')
branch_labels = None
depends_on = None

def upgrade():
    pass # upgrade code redacted

def downgrade():
    pass # downgrade code redacted

"""
Revision ID: b55e304ba00c
Revises: 7f5b06f81aef, 79ee02b5e895
Create Date: 2020-07-08 14:57:08.448885
"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
revision = 'b55e304ba00c'
down_revision = ('7f5b06f81aef', '79ee02b5e895')
branch_labels = None
depends_on = None

def upgrade():
    pass # upgrade code redacted

def downgrade():
    pass # downgrade code redacted

And the down revisions of 7f5b06f81aef and 79ee02b5e895.

The current revision is 7f5b06f81aef.

When I run alembic_runner.migrate_down_one() in a test I get the following error:

alembic.util.exc.CommandError: Destination 79ee02b5e895 is not a valid downgrade target from current head(s)

Which lead me to check alembic_runner.history.previous_revision(alembic_runner.current), and indeed it returns 79ee02b5e895. But as you can see that is not the previous revision, but a "sibling" of 7f5b06f81aef in b55e304ba00c.

Given the structure of my history, I would expect alembic_runner.history.previous_revision(alembic_runner.current) to return effc355fddff.

This behaviour worked on version 0.5.0, but after I've upgraded to 0.8.2 I started getting this error.

[DanCardin]

Perhaps I'm not understanding your migration history. I attempted to recreate it from the above descriptions here: https://github.com/schireson/pytest-alembic/tree/dc/55/branched-history-downgrade, but migrate_down_one from 7f5b06f81aef doesn't fail; so i assume i've gotten something wrong.

The code snippets reference some revisions which dont have corresponding code blocks. If you could perhaps give me a an ascii kind of thing specifying for the important revisions to this problem (like how alembic history shows minus all the extra stuff)

7f5b06f81aef, 79ee02b5e895 ->  b55e304ba00c
...

I can definitely see how we might have regressed. We had previously done history.raw_command("downgrade", "-1"). that was exceptionally slow, but left the determination of what was "-1" to alembic. I think we might be able to offload it back to alembic, but i'd like a proper failing test before i go changing things.

[olmokramer]

Thanks for the reply. I left out most of our history because it has more than 300 entries... Anyway, I've made a gist with the entire history and an excerpt containing all the relevant revisions here:

https://gist.github.com/olmokramer/e63dcef61b25c5b56c3001155f93c4c1

[DanCardin]

I recreated your shortened history in a test example, and wasn't able to recreate the Destination 79ee02b5e895 is not a valid downgrade target from current head(s) message for whatever reason.

I can however reproduce that with your history example, performing migrate_down_one in a loop will loop forever because it ignores the possibility of there being two down revisions (it's not obvious to me what the expected behavior would be).

I have some ideas of how it might be addressed, it's just very unfortunate that i can't reproduce the actual error you're hitting so i can't say for sure any fix i might come up with will work; if you have the opportunity and could maybe try to produce a minimalized example history in an examples/ example (like i did in the above branch), that'd be really convenient :P. (It might even just be my example test has the correct history, but not the equivalent test contents as you)

[olmokramer]

Thanks again for looking into this! I'll try to make a smaller example that triggers the error and let you know what I find early next week.

[DanCardin]

I altered the implementation of determining the history which helped in a different branched history scenario. I think I'd need a specific repro example for your history.

When I was thinking about how to get this to work in theory though, based on alembic's implementation of _walk, it's raised an exception about an ambiguous path when doing downgrade -1, so it's not clear to me that there's anything to do in at least the scenario i committed.

If you're writing a specific test and this isn't just running through the default set of tests, then you perhaps would need to explicitly downgrade to a given revision. If that's the case, I could likely capture the alembic exception and at the very least suggest as such in a re-reraise.

If it's one of the default tests failing, I could probably figure out a more general solution where it just directly downgrades to base, if it encounters this scenario.

EDIT: the above impl change is released in 0.8.3 btw

[DanCardin]

let us know if you can come up with a smaller reproducible example after 0.8.3. Otherwise i'm going to assume this is resolved due to lack of response/ability to reproduce.