- attached sqlalchemy-compileerr.patch
Clarify vague error from within DDLCompiler.visit_create_table
Currently, if I do something like declare a schema with a sqlalchemy.Unicode column type with no length, and try to use it with MySQL, I get an error like so:
sqlalchemy.exc.InvalidRequestError: VARCHAR requires a length on dialect mysql
Unfortunately, the trace looks like this:
justin@dev-ops:~/py27sqla/examples/ptah_simpleauth$ ../../bin/pserve settings.ini
Traceback (most recent call last):
File "../../bin/pserve", line 8, in <module>
load_entry_point('pyramid==1.3a3', 'console_scripts', 'pserve')()
File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 32, in main
return command.run()
File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 275, in run
relative_to=base, global_conf=vars)
File "/home/justin/py27sqla/lib/python2.7/site-packages/pyramid-1.3a3-py2.7.egg/pyramid/scripts/pserve.py", line 303, in loadapp
return loadapp(app_spec, name=name, relative_to=relative_to, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 247, in loadapp
return loadobj(APP, uri, name=name, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 272, in loadobj
return context.create()
File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 710, in create
return self.object_type.invoke(self)
File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/loadwsgi.py", line 146, in invoke
return fix_call(context.object, context.global_conf, **context.local_conf)
File "/home/justin/py27sqla/lib/python2.7/site-packages/PasteDeploy-1.5.0-py2.7.egg/paste/deploy/util.py", line 56, in fix_call
val = callable(*args, **kw)
File "/home/justin/py27sqla/examples/ptah_simpleauth/ptah_simpleauth/app.py", line 47, in main
Base.metadata.create_all()
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/schema.py", line 2515, in create_all
tables=tables)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 2234, in _run_visitor
conn._run_visitor(visitorcallable, element, **kwargs)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1904, in _run_visitor
**kwargs).traverse_single(element)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 86, in traverse_single
return meth(obj, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/ddl.py", line 67, in visit_metadata
self.traverse_single(table, create_ok=True)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 86, in traverse_single
return meth(obj, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/ddl.py", line 86, in visit_table
self.connection.execute(schema.CreateTable(table))
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1405, in execute
params)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1490, in _execute_ddl
compiled = ddl.compile(dialect=dialect)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/expression.py", line 1722, in compile
return self._compiler(dialect, bind=bind, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/schema.py", line 2852, in _compiler
return dialect.ddl_compiler(dialect, self, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 699, in __init__
self.string = self.process(self.statement)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 718, in process
return obj._compiler_dispatch(self, **kwargs)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 59, in _compiler_dispatch
return getter(visitor)(self, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/compiler.py", line 1377, in visit_create_table
not first_pk
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/dialects/mysql/base.py", line 1367, in get_column_specification
self.dialect.type_compiler.process(column.type)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 764, in process
return type_._compiler_dispatch(self)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/visitors.py", line 59, in _compiler_dispatch
return getter(visitor)(self, **kw)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/sql/compiler.py", line 1702, in visit_unicode
return self.visit_VARCHAR(type_)
File "/home/justin/py27sqla/lib/python2.7/site-packages/sqlalchemy/dialects/mysql/base.py", line 1654, in visit_VARCHAR
self.dialect.name)
sqlalchemy.exc.InvalidRequestError: VARCHAR requires a length on dialect mysql
With the attached patch, it looks more like:
sqlalchemy.exc.CompileError: In Table ptah_nodes, Column roles: VARCHAR requires a length on dialect mysql
This allows one to address this error without stab-in-the-dark guess-and-try cycles which are pretty miserable, and should help in resolving:
Comments (8)
-
Account Deleted -
repo owner Patches like these often give me a bad taste, since they are trying to take one very specific case where there's an unideal stack trace and add particular contextual information - but there's many other places where this same trace (such as in compiling a
cast()
), as well as other similar ones (any number of conditions that fail on the database side, for example), can be raised, where this information is not available. Getting it in one place is not really a substitute for the developer being able to usepdb.post_mortem()
, climb up the stack, and see the source of an error.The exceptions raised directly within
MySQLTypeCompiler
should probably also beCompileError
instances. It feels a little more appropriate forCompilerError
to be caught and re-raised with more information rather than totally changing the type of exception being emitted. That way the originator of the exception can make the choice that this exception is appropriate for being "contextualized" and I'd feel like this patch is not quite so arbitrary.Another decision to be made is the mechanics of the re-raise. This is the standard approach to a re-raise is:
except SomeException, e: # Py3K #raise NewException("message:" + str(e)) from e # Py2K raise NewException("message:" + str(e)), None, sys.exc_info()[2](2) # end Py2K
that at least preserves the original stack trace and in the case of Py3K provides both stack traces.
-
Account Deleted Fair enough, I didn't consider it a perfect patch, but I feel like there is some value to clarifying the case where "a schema declared somewhere in my code causes this traceback which is very mysterious".
I hope that we can at least agree the existing stack trace is highly frustrating and can only be solved with guesswork. It took a pretty good amount of time just to figure out where to put this patch.
It also happens to be one of the few places where a table and column name are particularly handy.
Perfect or not, I think it's tough to make an argument that sqlalchemy is better without these three lines of code.
-
repo owner - changed milestone to 0.7.5
will stick this in 0.7.5 for now keeping in mind it might get pushed out. An updated patch + a unit test in test/sql/test_compiler.py would expedite, though I can knock these out very quickly when I get to them.
-
Account Deleted I'm happy to write a test, if I can improve the patch, I'll do so - happy to work together on a patch that is more general, it took me a while to sort out this one.
-
repo owner sure, it involves emitting CompilerException from within the MySQL dialect, and it would also be nice to see if other dialects are doing something similar, then the code block in question would re-raise CompilerException only using the "reraise" format I have in my above comment. testing for exceptions uses the
assert_raises_message()
testing helper, most unit test modules should have a few examples of it. -
repo owner - changed status to resolved
09553dc90f4a95b314994b48068b046de1413104 contains the change to CompileException for all dialect-specific compilers, as well as the re-raise. I'm a tad concerned that changing the exception type in some of these places would not be backwards compatible for an app that is specifically catching InvalidRequestError, though this would be very unusual and I'd rather not wait til 0.8 for this change.
-
repo owner - removed milestone
Removing milestone: 0.7.5 (automated comment)
- Log in to comment
patch to emit clearer, more helpful error message