New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrading to implicit head that's already applied emits error message when it should likely pass silently, as is the case for normal heads already applied #336

Closed
sqlalchemy-bot opened this Issue Oct 29, 2015 · 6 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Oct 29, 2015

Migrated issue, originally created by Cedric Shock

alembic 0.8.0 fails upgrades to a head that another branch depends_on.

We are going to create the following revision graph.

base --- core 1 (core@head)
               \_____
                     \
base --- branch 1 --- branch 2 (branch@head)

Upgrading to core@head is successful until we upgrade to branch@head. After upgrading to branch@head, subsequent upgrades to core@head fail with the message Destination core@head is not a valid upgrade target from current head(s).

The following commands set up the desired revision graph.

alembic init alembic
alembic revision -m "core 1" --head=base --branch-label=core
alembic revision -m "branch 1" --head=base --branch-label=branch
alembic revision -m "branch 2" --head=branch@head --depends-on=core@head

Upgrading to core@head is successful multiple times in a row.

$ alembic upgrade core@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 50e7acdb8305, core 1
$ alembic upgrade core@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.

We can upgrade to branch@head.

$ alembic upgrade branch@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> f65508c78e3, branch 1
INFO  [alembic.runtime.migration] Running upgrade f65508c78e3, 50e7acdb8305 -> 1f50b22f2131, branch 2

But if we now try to upgrade to core@head again we get an error.

$ alembic upgrade core@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
ERROR [alembic.util.messaging] Destination core@head is not a valid upgrade target from current head(s)
  FAILED: Destination core@head is not a valid upgrade target from current head(s)

The error is a result of alembic.script.revision.RevisionMap.iterate_revisions raising RangeNotAncestorError.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 29, 2015

Michael Bayer (@zzzeek) wrote:

there's not a bug here I can see. Watch when we upgrade from total nothing to "heads" (please excuse my "python -m alembic.config" style, this is the same as just running "alembic"):

#!

$ python -m alembic.config upgrade heads
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 26e77aba1ddb, core 1
INFO  [alembic.runtime.migration] Running upgrade  -> 1a6fb30f89fa, branch 1
INFO  [alembic.runtime.migration] Running upgrade 1a6fb30f89fa, 26e77aba1ddb -> 2543a166cab2, branch 2

now we're at the branch 2 head. Notice that to get there, we had to apply "core 1". Core 1 is already applied. What would it mean to go there again?

Just run "heads" - both heads are there:

#!


$ python -m alembic.config heads        
2543a166cab2 (branch) (head)
26e77aba1ddb (core) (effective head)

"core" is known as the "effective head" because it is behind the branch, but if you were to make another version dependent on it, you'd be able to go to that as a head no problem:

#!

$ python -m alembic.config revision -m "core number 2" --head=core@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
  Generating /Users/classic/dev/alembic/foo/versions/3221600802a7_core_number_2.py ... done
$ python -m alembic.config upgrade core@head
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 26e77aba1ddb -> 3221600802a7, core number 2
$ python -m alembic.config heads            
2543a166cab2 (branch) (head)
3221600802a7 (core) (head)

the "effective head" term is discussed in http://alembic.readthedocs.org/en/latest/branches.html#branch-dependencies:

The head file that we used as a “dependency”, 55af2cb1c267, is displayed as an “effective” head, which we can see also in the history display earlier. What this means is that at the moment, if we were to upgrade all versions to the top, the 55af2cb1c267 revision number would not actually be present in the alembic_version table; this is because it does not have a branch of its own subsequent to the 2a95102259be revision which depends on it.

please confirm no issue on your end thanks!

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 29, 2015

Michael Bayer (@zzzeek) wrote:

I guess we could say the bug is that it should pass without an error message. So yes, that might be considered a fairly minor bug. I originally thought the issue was that the movement couldn't be made which would be a lot more serious.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 29, 2015

Changes by Michael Bayer (@zzzeek):

  • changed title from "Branching causes "Destination is not a valid upgra" to "upgrading to implicit head that's already applied "
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 29, 2015

Changes by Michael Bayer (@zzzeek):

  • set milestone to "fasttrack"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 19, 2016

Michael Bayer (@zzzeek) wrote:

Don't raise RangeNotAncestor for sibling branches

Fixed bug where upgrading to the head of a branch which is already
present would fail, only if that head were also the dependency
of a different branch that is also upgraded, as the revision system
would see this as trying to go in the wrong direction. The check
here has been refined to distinguish between same-branch revisions
out of order vs. movement along sibling branches.

When we're about to claim an error due to
"alembic upgrade greater to lower", make sure this
isn't a request to hit a node in a different branch
that's already implied.

Change-Id: I8641162bb05c6226f0ea12b88b548df41f5a6b51
Fixes: #336

2df9c52

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 19, 2016

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment