Skip to content

Commit

Permalink
Add test and fix for SA Engine repr failure(#130)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
elbenfreund committed Jun 28, 2016
1 parent 6e30daf commit 72144bc
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 3 deletions.
2 changes: 1 addition & 1 deletion hamster_lib/backends/sqlalchemy/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def __init__(self, config, session=None):
# we receive a session. Should be require the session to bring its own
# engine?
engine = create_engine(self._get_db_url())
self.logger.debug(_("Engine '{}' created.".format(engine)))
self.logger.debug(_('Engine created.'))
objects.metadata.bind = engine
objects.metadata.create_all(engine)
self.logger.debug(_("Database tables created."))
Expand Down
5 changes: 4 additions & 1 deletion hamster_lib/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@
}

# See: https://wiki.python.org/moin/PortingToPy3k/BilingualQuickRef#gettext
# [FIXME]
# Is this correct? http://www.wefearchange.org/2012/06/the-right-way-to-internationalize-your.html
# seems to user ``sys.version_info.major > 3``
kwargs = {}
if sys.version_info < (3,):
if sys.version_info.major < 3:
kwargs['unicode'] = True
gettext.install('hamster-lib', **kwargs)

Expand Down
15 changes: 15 additions & 0 deletions tests/backends/sqlalchemy/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from __future__ import unicode_literals

import datetime
import os

import fauxfactory
import pytest
Expand Down Expand Up @@ -51,6 +52,20 @@ def fin():
request.addfinalizer(fin)


@pytest.fixture(params=[
fauxfactory.gen_utf8(),
fauxfactory.gen_alphanumeric(),
':memory:',
])
def db_path_parametrized(request, tmpdir):
"""Parametrized database paths."""
if request.param == ':memory:':
path = request.param
else:
path = os.path.join(tmpdir.strpath, request.param)
return path


@pytest.fixture
def alchemy_config(request, base_config):
"""Provide a config that is suitable for sqlalchemy stores."""
Expand Down
8 changes: 7 additions & 1 deletion tests/backends/sqlalchemy/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

import pytest
from hamster_lib.backends.sqlalchemy import (AlchemyActivity, AlchemyCategory,
AlchemyFact)
AlchemyFact, SQLAlchemyStore)


# The reason we see a great deal of count == 0 statements is to make sure that
# db rollback works as expected. Once we are confident in our sqlalchemy/pytest
# setup those are not really needed.


class TestStore(object):
"""Tests to make sure our store/test setup behaves as expected."""
def test_build_is_not_persistent(self, alchemy_store, alchemy_category_factory):
Expand Down Expand Up @@ -61,6 +62,11 @@ def test_get_db_url_missing_keys(self, alchemy_config_missing_store_config_param
with pytest.raises(ValueError):
alchemy_store._get_db_url()

def test_init_with_unicode_path(self, alchemy_config, db_path_parametrized):
"""Test that Instantiating a store with a unicode path works."""
alchemy_config['db_path'] = db_path_parametrized
assert SQLAlchemyStore(alchemy_config)


class TestCategoryManager():
def test_add_new(self, alchemy_store, alchemy_category_factory):
Expand Down

0 comments on commit 72144bc

Please sign in to comment.