Alembic: investigate bytes being passed to revision logic when use_unicode=0 for pymysql / other mysql drivers

[a372661369]

Python3.6
alembic == 1.0.8

Traceback (most recent call last):
File "//bin/alembic", line 11, in
load_entry_point('alembic==1.0.8', 'console_scripts', 'alembic')()
File "/lib/python3.6/site-packages/alembic/config.py", line 527, in main
CommandLine(prog=prog).main(argv=argv)
File "/lib/python3.6/site-packages/alembic/config.py", line 521, in main
self.run_cmd(cfg, options)
File "/lib/python3.6/site-packages/alembic/config.py", line 501, in run_cmd
dict((k, getattr(options, k, None)) for k in kwarg)
File "/lib/python3.6/site-packages/alembic/command.py", line 276, in upgrade
script.run_env()
File "/lib/python3.6/site-packages/alembic/script/base.py", line 475, in run_env
util.load_python_file(self.dir, "env.py")
File "/lib/python3.6/site-packages/alembic/util/pyfiles.py", line 90, in load_python_file
module = load_module_py(module_id, path)
File "/lib/python3.6/site-packages/alembic/util/compat.py", line 156, in load_module_py
spec.loader.exec_module(module)
File "", line 678, in exec_module
File "", line 219, in _call_with_frames_removed
File "alembic_scripts/env.py", line 70, in
run_migrations_online()
File "alembic_scripts/env.py", line 65, in run_migrations_online
context.run_migrations()
File "", line 8, in run_migrations
File "/lib/python3.6/site-packages/alembic/runtime/environment.py", line 839, in run_migrations
self.get_context().run_migrations(kw)
File "/lib/python3.6/site-packages/alembic/runtime/migration.py", line 350, in run_migrations
for step in self._migrations_fn(heads, self):
File "/lib/python3.6/site-packages/alembic/command.py", line 265, in upgrade
return script._upgrade_revs(revision, rev)
File "/lib/python3.6/site-packages/alembic/script/base.py", line 360, in _upgrade_revs
revs = list(revs)
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 740, in _iterate_revisions
requested_lowers = self.get_revisions(lower)
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 318, in get_revisions
return sum([self.get_revisions(id_elem) for id_elem in id_], ())
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 318, in
return sum([self.get_revisions(id_elem) for id_elem in id_], ())
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 323, in get_revisions
for rev_id in resolved_id
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 323, in
for rev_id in resolved_id
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 378, in _revision_for_ident
for x in self._revision_map
File "/lib/python3.6/site-packages/alembic/script/revision.py", line 379, in
if x and x.startswith(resolved_id)
TypeError: startswith first arg must be str or a tuple of str, not int

[zzzeek]

hi there -

can you please share some detail about what command you are running, what the arguments are, as well as how are you building migration files? did you modify alembic.ini.mako or hand-write any integer values which should be strings?

[zzzeek]

are you running the "upgrade()" function programmatically?

[a372661369]

when i running "alembic upgrade", the function "get_revisions" of the parameter "id_" is (b'63195b488b5e',), and then run the "self._resolve_revision_number(id_)" function, return "
util.to_tuple(id_, default=None)" , but the id_ is binary_type not string_types, so return tuple(x) like "(54, 51, 49, 57, 53, 98, 52, 56, 56, 98, 53, 101)"

/alembic/scripts/revision.py
```
def get_revisions(self, id_):
if isinstance(id_, (list, tuple, set, frozenset)):
return sum([self.get_revisions(id_elem) for id_elem in id_], ())
else:
resolved_id, branch_label = self._resolve_revision_number(id_)
return tuple(
self._revision_for_ident(rev_id, branch_label)
for rev_id in resolved_id
)

def _resolve_revision_number(self, id_):

    if isinstance(id_, compat.string_types) and "@" in id_:
        branch_label, id_ = id_.split("@", 1)
    else:
        branch_label = None

    # ensure map is loaded
    self._revision_map
    if id_ == "heads":
        if branch_label:
            return (
                self.filter_for_lineage(self.heads, branch_label),
                branch_label,
            )
        else:
            return self._real_heads, branch_label
    elif id_ == "head":
        current_head = self.get_current_head(branch_label)
        if current_head:
            return (current_head,), branch_label
        else:
            return (), branch_label
    elif id_ == "base" or id_ is None:
        return (), branch_label
    else:
        return util.to_tuple(id_, default=None), branch_label

def to_tuple(x, default=None):
if x is None:
return default
elif isinstance(x, string_types):
return (x,)
elif isinstance(x, collections_abc.Iterable):
return tuple(x)
else:
return (x,)
```
@zzzeek

[zzzeek]

hi -

unfortunately I cannot reproduce this with python 3.6, alembic 1.0.8:

[classic@photon2 alembic]$ .venv/bin/python -V
Python 3.6.8
[classic@photon2 alembic]$ .venv/bin/python -c "import alembic; print(alembic.__version__)"
1.0.8

Try with the exact version number you gave, which I dont have locally, returns correct error:

[classic@photon2 alembic]$ .venv/bin/alembic  upgrade 63195b488b5e
ERROR [alembic.util.messaging] Can't locate revision identified by '63195b488b5e'
  FAILED: Can't locate revision identified by '63195b488b5e'

Try with the version that's actually in this env, no error:

[classic@photon2 alembic]$ .venv/bin/alembic  upgrade 71bb196591f2
[classic@photon2 alembic]$ .venv/bin/alembic  heads
71bb196591f2 (head)

please share your complete environment including env.py and any migration files involved thanks!

[zzzeek]

additionally:

diff --git a/alembic/script/revision.py b/alembic/script/revision.py
index af08688..d4cec37 100644
--- a/alembic/script/revision.py
+++ b/alembic/script/revision.py
@@ -314,6 +314,13 @@ class RevisionMap(object):
         full revision.

         """
+        assert not isinstance(id_, bytes)
+        assert isinstance(id_, (tuple, str))
+        if isinstance(id_, tuple):
+            for elem in id_:
+                assert isinstance(elem, str)
+                assert not isinstance(elem, bytes)
+
         if isinstance(id_, (list, tuple, set, frozenset)):
             return sum([self.get_revisions(id_elem) for id_elem in id_], ())
         else:

these all pass. the id_ is not bytes. please illustrate how you are invoking this command.

[zzzeek]

if there were some known issue say in a particular version of argparse, that could be at fault here but it would likely be something that's reported much more widely.

[a372661369]

thank you for your question. i solution this problem. it cause by the function "get_current_heads", and in alembic.ini i set config "sqlalchemy.url = mysql+pymysql://root:root@localhost/database?charset=utf8&use_unicode=0". when i set use_unicode=1, it's ok. if not set this config, the sqlalchemy will return bytes

[zzzeek]

OK you shouldn't use use_unicode at all, just leave it at the default. I'll see why exactly it has this effect though because SQLAlchemy should be decoding the bytes.

[zzzeek]

OK so yes, this is because under python3 you cannot set the use_unicode=0 flag ever. Under Python 2, SQLAlchemy just gets these as strings and things are fine but under py3k, no good. let me consider adding an assertion.

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the master branch:

Raise for non-string revision identifier https://gerrit.sqlalchemy.org/1202