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

[#130]Add test and fix for SA Engine repr failure #158

Merged
merged 1 commit into from
Jun 28, 2016

Conversation

elbenfreund
Copy link
Collaborator

Running python 2, it apears SQLAlchemy is not fully capable to handle
unicode strings as urls. While they can be passed and will be processed
(correctly as far as we can tell) there are issues with 'representing'
an Engine or sqlalchemy.engine.url.URL. As far as we understand,
their repr() call will naivly construct a str from its various
components. If any of those substrings (like the URL) happens to be an
unicode string, this will fail if it can not be ascii-decoded.
SQLAlchemy should propably call repr(url) instead.

For now, this commit just avoids the problem entirely by amending the
triggering log message not to render a string representation of the
created engine. A proper solution would probably be to write a custom
wrapper function that handles Engine representation properly.

Also, a test that triggers the origianl offending behaviour has been
added, so any regression should be easier to find.

Closes: #130

Running python 2, it apears SQLAlchemy is not fully capable to handle
unicode strings as urls. While they can be passed and will be processed
(correctly as far as we can tell) there are issues with 'representing'
an ``Engine`` or ``sqlalchemy.engine.url.URL``. As far as we understand,
their repr() call will naivly construct a str from its various
components. If any of those substrings (like the URL) happens to be an
unicode string, this will fail if it can not be ascii-decoded.
SQLAlchemy should propably call repr(url) instead.

For now, this commit just avoids the problem entirely by amending the
triggering log message not to render a string representation of the
created engine. A proper solution would probably be to write a custom
wrapper function that handles Engine representation properly.

Also, a test that triggers the origianl offending behaviour has been
added, so any regression should be easier to find.

Closes: #130
@elbenfreund elbenfreund merged commit 72144bc into develop Jun 28, 2016
@elbenfreund elbenfreund deleted the bugfix/130-Engine_with_unicode_path branch June 28, 2016 21:36
@elbenfreund elbenfreund modified the milestone: 0.11.0 Jul 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant