Fixed bug when using a declarative function for the __tablename__ #592
Conversation
When specifying a function to dynamically define a tablename as a declared attribute it became impossible to overwrite it using a manually defined name. Look at the test to see the issue, with the original code this test would fail.
Pull Request Test Coverage Report for Build 1966
💛 - Coveralls |
gino/declarative.py
Outdated
| if getattr(sub_cls, '__tablename__', None) and \ | ||
| not callable(getattr(sub_cls, '__tablename__')): | ||
| table_name = getattr(sub_cls, '__tablename__') |
There was a problem hiding this comment.
Good catch!
But this still doesn't cover all cases, though most likely corner cases. We should move it into the loop.
Could you help to do that?
There was a problem hiding this comment.
Hi, I can. But which case isn't caught using this? I agree it's nicer to have it in the loop anyways, but I think it's probably good to write another test for any uncaught situation.
There was a problem hiding this comment.
For example, one of the parent classes or mixins specify __tablename__ as a variable
There was a problem hiding this comment.
Ah I see, that makes sense. I added a test for that case as well. Maybe I should also have added some tests that are supposed to throw errors on faulty behaviour... but I think it's good to go like this. Can this be merged and pushed to pypi? And maybe #590 can be merged as well?
This test ensures that the __tablename__ is set correctly according to the inheritance order of the mixins.
|
Thanks |
3 participants
When specifying a function to dynamically define a tablename as a declared
attribute it became impossible to overwrite it using a manually defined name.
Look at the test to see the issue, with the original code this test would fail.