Add editor entity and use sqlalchemy-dst to automatically serialize it #20
Conversation
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Also what's up with that Jenkins spam? |
|
@brainzbot add to whitelist |
No idea, never got the time to check. |
There was a problem hiding this comment.
I get this error when testing this code:
>>> from brainzutils import musicbrainz_db
>>> musicbrainz_db.init_db_engine("postgresql://musicbrainz:musicbrainz@musicbrainz_db:5432/musicbrainz_db"
... )
>>> from brainzutils.musicbrainz_db import editor
>>> editor.get_editor_by_id(1)
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/strategy_options.py", line 122, in _generate_path
attr = getattr(path.entity.class_, attr)
AttributeError: type object 'Editor' has no attribute 'type'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/code/brainzutils-python/brainzutils/musicbrainz_db/editor.py", line 17, in get_editor_by_id
editor = _get_editor_by_id(editor_id)
File "/code/brainzutils-python/brainzutils/musicbrainz_db/editor.py", line 24, in _get_editor_by_id
includes=[],
File "/code/brainzutils-python/brainzutils/musicbrainz_db/editor.py", line 42, in fetch_multiple_editors
options(joinedload("type"))
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line 1324, in options
return self._options(False, *args)
File "<string>", line 2, in _options
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/base.py", line 201, in generate
fn(self, *args[1:], **kw)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line 1341, in _options
opt.process_query(self)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/strategy_options.py", line 83, in process_query
self._process(query, True)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/strategy_options.py", line 314, in _process
val._bind_loader(query, query._attributes, raiseerr)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/strategy_options.py", line 407, in _bind_loader
loader.path, token, None, raiseerr)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/strategy_options.py", line 128, in _generate_path
attr, path.entity)
sqlalchemy.exc.ArgumentError: Can't find property named 'type' on the mapped entity Mapper|Editor|editor in this Query.
| @@ -197,10 +198,17 @@ def serialize_url(url, includes=None): | |||
| return data | |||
|
|
|||
|
|
|||
| def serialize_editors(editor, includes=None): | |||
| data = row2dict(editor, exclude_pk=True, exclude={'password', 'ha1'}) | |||
There was a problem hiding this comment.
nitpick, but just returning the result of row2dict here without assigning it to data would be cleaner.
There was a problem hiding this comment.
Yeah, I intentionally left it like this because the includes will have to be added to data before it's returned. I added a comment explaining that.
requirements.txt
Outdated
| @@ -8,3 +8,4 @@ msgpack-python==0.4.8 | |||
| requests==2.18.1 | |||
| SQLAlchemy==1.2.5 | |||
| mbdata==2017.6.2 | |||
| sqlalchemy-dst | |||
There was a problem hiding this comment.
please specify a version number here.
| @@ -31,6 +31,7 @@ | |||
| "artists", "labels", "recordings", "release-groups", "media", "annotation", "aliases" | |||
| ] + TAG_INCLUDES + RELATION_INCLUDES, | |||
| 'artist': ["recordings", "releases", "media", "aliases", "annotation"] + RELATION_INCLUDES + TAG_INCLUDES, | |||
| 'editor': [], # TODO: Add editor includes | |||
There was a problem hiding this comment.
Should we add a ticket for this?
| editor = mb_editor.get_editor_by_id(2323) | ||
| self.assertDictEqual(editor, self.editor_1_dict) | ||
|
|
||
| def test_fetch_multiple_artists(self): |
There was a problem hiding this comment.
I guess this should be test_fetch_multiple_editors?
|
Well this is kinda embarrassing, my code was completely broken but totally slipped through the tests. I'll figure out why the unit tests don't work now and add some integration tests using a real database to prevent this from happening again. |
This is part of BU-17.
After looking at a couple libraries for serializing SQLAlchemy rows to dicts, sqlalchemy-dst seems like a well documented and still maintained library.
In this PR I've added the editor entity and the respective tests to demonstrate automatic serialization.