Skip to content
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

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

Closed
a372661369 opened this issue Apr 2, 2019 · 10 comments

Comments

Projects
None yet
3 participants
@a372661369
Copy link

commented Apr 2, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

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

@zzzeek zzzeek added the question label Apr 2, 2019

@a372661369

This comment has been minimized.

Copy link
Author

commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Author

commented Apr 24, 2019

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

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

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 zzzeek changed the title when i get the version, raise TypeError: startswith first arg must be str or a tuple of str, not int investigate bytes being passed to revision logic when use_unicode=0 for pymysql / other mysql drivers Apr 24, 2019

@zzzeek

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

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

This comment has been minimized.

Copy link
Collaborator

commented Apr 24, 2019

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

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

@zzzeek zzzeek removed the question label Apr 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.