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

Many-to-Many relationships broken on SQLAlchemy>1.3.5 #2099

Closed
ix5 opened this issue Mar 22, 2021 · 8 comments
Closed

Many-to-Many relationships broken on SQLAlchemy>1.3.5 #2099

ix5 opened this issue Mar 22, 2021 · 8 comments

Comments

@ix5
Copy link
Contributor

ix5 commented Mar 22, 2021

Take the following model:

entry_tags_table = db.Table(
    "entry_tags",
    db.Model.metadata,
    db.Column("entry_id", db.Integer, db.ForeignKey("entry.id")),
    db.Column("tag_id", db.Integer, db.ForeignKey("tag.id")),
)
class Entry(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    title = db.Column(db.String)
    tags = db.relationship("Tag", secondary=entry_tags_table)

class Tag(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.Unicode, nullable=False)
    entries = db.relationship("Entry", secondary=entry_tags_table)

[...]
class DefaultView(sqla.ModelView):
    column_searchable_list = [
        "title",
        "tags.name",
    ]

admin = admin.Admin(app, name='Test', template_mode='bootstrap4', index_view=DefaultView(Entry, db.session))

This will throw an error on SQLALchemy 1.4.x

/home/me/.virtualenvs/vp/lib/python3.9/site-packages/sqlalchemy/orm/relationships.py:3435: SAWarning:
relationship 'Tag.entries' will copy column tag.id to column entry_tags.tag_id,
which conflicts with relationship(s): 'Entry.tags' (copies tag.id to entry_tags.tag_id).
If this is not the intention, consider if these relationships should be linked with back_populates,
or if viewonly=True should be applied to one or more if they are read-only.
For the less common case that foreign key constraints are partially overlapping,
the orm.foreign() annotation can be used to isolate the columns that should be written towards.
The 'overlaps' parameter may be used to remove this warning.

Adding back_populates or backrefs as suggested by the error message and as documented in Basic Relationship Patterns like this:

[...]
    entries = db.relationship(Entry, secondary=entry_tags_table, back_populates="tags")
[...]
    tags = db.relationship(Tag, secondary=entry_tags_table, back_populates="entries")

results in a new error:

Traceback (most recent call last):
  File "/home/me/data/sixfive/main.py", line 24, in <module>
    from admin import admin
  File "/home/me/data/sixfive/admin/__init__.py", line 1, in <module>
    from .view import admin
  File "/home/me/data/sixfive/admin/view.py", line 727, in <module>
    index_view=DefaultView(Entry, db.session, url="/admin"),
  File "/home/me/data/sixfive/admin/view.py", line 45, in __init__
    super(DefaultView, self).__init__(model, session, *args, **kwargs)
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/view.py", line 327, in __init__
    super(ModelView, self).__init__(model, name, category, endpoint, url, static_folder,
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/model/base.py", line 818, in __init__
    self._refresh_cache()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/model/base.py", line 913, in _refresh_cache
    self._search_supported = self.init_search()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/view.py", line 581, in init_search
    if tools.is_hybrid_property(self.model, name):
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/tools.py", line 215, in is_hybrid_property
    return last_name in get_hybrid_properties(last_model)
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/tools.py", line 195, in get_hybrid_properties
    for key, prop in inspect(model).all_orm_descriptors.items()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/sqlalchemy/inspection.py", line 71, in inspect
    raise exc.NoInspectionAvailable(
sqlalchemy.exc.NoInspectionAvailable: No inspection system is available for object of type <class 'str'>
make: *** [Makefile:22: runserver] Error 1

The call to inspect() fails at is_hybrid_property() for the attr_name tags.name


Link to gist for reproducing: app.py

Library versions:

Flask-Admin      1.5.7
Flask-SQLAlchemy 2.5.1
SQLAlchemy       1.4.2

Possibly related:

@mrjoes mrjoes closed this as completed Mar 22, 2021
@mrjoes mrjoes reopened this Mar 22, 2021
@mrjoes
Copy link
Contributor

mrjoes commented Mar 22, 2021

There are few things that are not working with SQLAlchemy 1.4:

  1. sqlalchemy-utils is not compatible:

File "/home/travis/build/flask-admin/flask-admin/.tox/py37-WTForms2/lib/python3.7/site-packages/sqlalchemy_utils/functions/orm.py", line 14, in
from sqlalchemy.orm.query import _ColumnEntity

More here: kvesteri/sqlalchemy-utils#505

  1. I'm checking what's going with the is_hybrid_property. There was a suggested change from the SQLAlchemy author - will see if it helps.

@mrjoes
Copy link
Contributor

mrjoes commented Mar 22, 2021

I tried to reproduce the M2M issue with the latest SQLAlchemy 1.4.2 and it works as expected with and without back_populates.

Here's a GIST: https://gist.github.com/mrjoes/30cf8cd7865484ade357e966ad82439d

sqlalchemy-utils is still a problem, our tests are failing.

@ix5
Copy link
Contributor Author

ix5 commented Mar 22, 2021

Sorry, reproduction was missing a crucial bit: column_searchable_list is where it fails to resolve.
See gist: app.py

@mrjoes
Copy link
Contributor

mrjoes commented Mar 22, 2021

Does not reproduce even if I add column_searchable_list. Your gist does not start due to ValueError: urls must start with a leading slash

image

Just in case - this is latest master branch.

@ix5
Copy link
Contributor Author

ix5 commented Mar 23, 2021

I did not run master branch, but rather this constellation:

Flask-Admin      1.5.7
Flask-SQLAlchemy 2.5.1
SQLAlchemy       1.4.2

(flask-admin 1.5.7 is the latest in pypi)

If the master branch indeed fixes the issue, a new release should be made.

@alexa-infra
Copy link
Contributor

is_hybrid_property has been fixed recently in #2098

@ix5
Copy link
Contributor Author

ix5 commented Mar 23, 2021

Guess this fixes it then.

@ix5 ix5 closed this as completed Mar 23, 2021
@ersanjay5991
Copy link

If this is not the intention, consider if these relationships should be linked with back_populates, flask sqlalchemy

I change to model relation

users = db.relationship('User', secondary=association_table,back_populates="events")

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants