fix: change sqla is_relationship for support sqla<=2.0.0 #2357
Conversation
|
how about this PR? > @flask-admin maintainer |
|
Is it possible to merge this PR? Relationships are not fully usable in SQLA2 currently |
There was a problem hiding this comment.
In #2332 there is a commit aee829a that adds model.registry.configure(). This is described in https://docs.sqlalchemy.org/en/20/orm/mapping_api.html#sqlalchemy.orm.registry.configure.
I'm no SQLAlchemy expert, only saying that this stopped the test exceptions and looks like it works with the example in the linked issue.
| @@ -224,7 +224,8 @@ def is_hybrid_property(model, attr_name): | |||
|
|
|||
|
|
|||
| def is_relationship(attr): | |||
| return hasattr(attr, 'property') and hasattr(attr.property, 'direction') | |||
| return (hasattr(attr, 'property') and hasattr(attr.property, 'direction') # sqla<2.0.0 | |||
| or (hasattr(attr, '_is_relationship') and attr._is_relationship)) # sqla>=2.0.0 | |||
There was a problem hiding this comment.
Are the attributes starting with _ documented?
There was a problem hiding this comment.
It seems this is not documented.
Actually I just found _is_relationship when debugging. this is reason why I use _is_relationship.
if there is a solution that documented (model.registry.configure()), the solution is best!
There was a problem hiding this comment.
Safer I guess anyway, better or best? I found another way #2398. Replacing mapper.iterate_properties with mapper.attrs, which does a check to see if model.registry.configure() has been done. But that does seem to have been around for a while, why iterate_properties was used in the first place I do not know.
There was a problem hiding this comment.
I'm also not familiar with SQLAlchemy, a just user of flask-admin, sqlalchemy. So I follow mantainer's decistion. ;)
|
@cjmayo do we still need this for sqlalchemy 1/2 compatibility? |
|
I think the merged #2398 has addressed this problem. |
close #2345