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

InvalidRequestError: Class does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class. #473

Closed
MichaelPereira opened this issue Feb 27, 2017 · 12 comments
Milestone

Comments

@MichaelPereira
Copy link

Hi,

First of all congrats on the new release! This will allow us to go back to a pinned version of flask-sqlalchemy instead of the master branch 👍

On the other hand, the change #467 now generates this stack-trace in the Flask-AppBuilder project which I would need some help investigating to be able to use release 2.2 directly:

12:48:03   File "/src/<module_name>/models.py", line 10, in <module>
12:48:03     from flask_appbuilder.security.sqla.models import User
12:48:03   File "/usr/local/lib/python2.7/dist-packages/flask_appbuilder/security/sqla/models.py", line 12, in <module>
12:48:03     class Permission(Model):
12:48:03   File "/usr/local/lib/python2.7/dist-packages/flask_sqlalchemy/__init__.py", line 602, in __init__
12:48:03     DeclarativeMeta.__init__(self, name, bases, d)
12:48:03   File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/api.py", line 55, in __init__
12:48:03     _as_declarative(cls, classname, cls.__dict__)
12:48:03   File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.py", line 88, in _as_declarative
12:48:03     _MapperConfig.setup_mapping(cls, classname, dict_)
12:48:03   File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.py", line 103, in setup_mapping
12:48:03     cfg_cls(cls_, classname, dict_)
12:48:03   File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.py", line 133, in __init__
12:48:03     self._setup_inheritance()
12:48:03   File "/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.py", line 429, in _setup_inheritance
12:48:03     "table-mapped class." % cls
12:48:03 InvalidRequestError: Class <class 'flask_appbuilder.security.sqla.models.Permission'> does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class.

The error message contradicts the model declaration as it does have a __tablename defined:

class Permission(Model):
    __tablename__ = 'ab_permission'
    id = Column(Integer, Sequence('ab_permission_id_seq'), primary_key=True)
    name = Column(String(100), unique=True, nullable=False)

    def __repr__(self):
        return self.name

I understand that this change is related to the @declared_attr decorator but despite reading http://docs.sqlalchemy.org/en/latest/orm/extensions/declarative/api.html#api-reference I don't really know exactly how I should apply it to the model.

Thanks for your help

@davidism
Copy link
Member

BoundDeclarativeMeta must be paired with a subclass of flask_sqlalchemy.Model. Is there a reason you're not using db.Model? If nothing else, your base model needs to inherit from flask_sqlalchemy.Model.

@dusktreader
Copy link

@davidism This does seem to be an issue. Using a tablename declared_attr from a 'mixin' does not work:

    def test_tablename_mixin(self):
        app = flask.Flask(__name__)
        db = fsa.SQLAlchemy(app)

        class Duck(db.Model):
            __abstract__ = True

        class NameMixin():

            @declared_attr
            def __tablename__(cls):
                return 'drake_mallard'

        class Darkwing(Duck, NameMixin):
            id = db.Column(db.Integer, primary_key=True)

        class Donald(Duck):
            id = db.Column(db.Integer, primary_key=True)

        self.assertEqual(Darkwing.__tablename__, 'drake_mallard')
        self.assertEqual(Donald.__tablename__, 'donald')

Results in the following test failure:

======================================================================
FAIL: test_tablename_mixin (__main__.TablenameTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_sqlalchemy.py", line 359, in test_tablename_mixin
    self.assertEqual(Darkwing.__tablename__, 'drake_mallard')
AssertionError: 'darkwing' != 'drake_mallard'
- darkwing
+ drake_mallard

I do notice in the execution the following error is spawned:

sqlalchemy/ext/declarative/api.py:182: SAWarning: Unmanaged access of declarative attribute __tablename__ from non-mapped class Base
  (desc.fget.__name__, cls.__name__))

Perhaps I'm using my name mixin wrong?

@dusktreader
Copy link

@davidism it also appears that the solution that I had previously used in my own project, monkey patching _should_set_tablename with a lambda that always returns false, does not work any more and renders the following error:

sqlalchemy.exc.InvalidRequestError: Class <class MyClass > does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class.
ERROR: could not load /some/path/test/conftest.py

@dusktreader
Copy link

@davidism @MichaelPereira
I think I might have found something. The ordering of the base classes is important (now). If you add your name-mixin class as the first base-class, the error goes away. For the test case I posted above, changing to:

    def test_tablename_mixin(self):
        app = flask.Flask(__name__)
        db = fsa.SQLAlchemy(app)

        class Duck(db.Model):
            __abstract__ = True

        class NameMixin():

            @declared_attr
            def __tablename__(cls):
                return 'drake_mallard'

        class Darkwing(NameMixin, Duck):
            id = db.Column(db.Integer, primary_key=True)

        class Donald(Duck):
            id = db.Column(db.Integer, primary_key=True)

        self.assertEqual(Darkwing.__tablename__, 'drake_mallard')
        self.assertEqual(Donald.__tablename__, 'donald')

eliminates the problem. This isn't a desirable solution, however.

@davidism
Copy link
Member

davidism commented Feb 28, 2017

The order of base classes has always been significant in Python, bases are scanned left to right. So I'm not going to consider that part a bug.

__tablename__ being ignored in mixins is a bug and is already reported in #470.

I'm fairly sure this issue has to do with their use of a completely separate Model.

@davidism
Copy link
Member

davidism commented Mar 1, 2017

@MichaelPereira did you have a chance to try out my initial suggestion? I'd like to know if this is actually a bug so I can work on 2.2.1 over the weekend.

@ThiefMaster
Copy link
Contributor

I'm quite sure it is a bug, it also fails in my application in this model - and no mixins are involved there: https://github.com/indico/indico/blob/master/indico/modules/events/registration/models/items.py

@davidism
Copy link
Member

davidism commented Mar 1, 2017

I'm going to need a minimal example. These projects are nice but it's hard to trace what's going on. Just to confirm, you're saying this is a different bug, not a duplicate of the other __tablename__ issues? From my initial look at this bug, the op is using a completely independent base model that does not have the expected interface.

Their base model class should look like

from flask_sqlalchemy import Model as _BaseModel

# no as_declarative here, that's what make_declarative_base does
# or make_declarative_base should be overridden to return the as_declarative version
class Model(_BaseModel):
    ...

@MichaelPereira
Copy link
Author

@davidism thanks for looking into the problem. I'm sorry for not getting back earlier as we got swamped with the S3 outage in the meantime. I will try to get a reproducible example out of it since our tool is not open-source to help the resolution of the bug.

@davidism
Copy link
Member

davidism commented Mar 2, 2017

@MichaelPereira oh, I was under the impression that your project was Flask-AppBuilder, since you linked to its source. The issue appears to be there, although an example is still useful.

MichaelPereira added a commit to MichaelPereira/Flask-AppBuilder that referenced this issue Mar 4, 2017
@MichaelPereira
Copy link
Author

MichaelPereira commented Mar 4, 2017

I contributed a little to it, but our project builds upon it. Anyway, if it's fixed for Flask-AppBuilder, it will be fixed for everyone using it :)

It took me a few hours but I was able to get a reproducible error with just the bare minimum. Here are the steps:

  • clone https://github.com/MichaelPereira/Flask-AppBuilder.git
  • create a docker image from that repo: docker build -t . Note the docker container id at the end
  • go to the registration_user example folder: cd examples/user_registration/
  • run the docker image with that example: docker run -v $(pwd):/app <container_id> python /app/run.py
    You should get the following stack-trace:
$ docker run -v `pwd`:/app 1741cdc3a399  python /app/run.py
/usr/local/lib/python3.5/site-packages/flask_sqlalchemy/__init__.py:839: FSADeprecationWarning: SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and will be disabled by default in the future.  Set it to True or False to suppress this warning.
  'SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and '
Traceback (most recent call last):
  File "/app/run.py", line 1, in <module>
    from app import app
  File "/app/app/__init__.py", line 16, in <module>
    appbuilder = AppBuilder(app, db.session)
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/base.py", line 130, in __init__
    self.init_app(app, session)
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/base.py", line 147, in init_app
    from flask_appbuilder.security.sqla.manager import SecurityManager
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/security/sqla/manager.py", line 7, in <module>
    from .models import User, Permission, PermissionView, RegisterUser, ViewMenu, Role
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/security/sqla/models.py", line 12, in <module>
    class Permission(Model):
  File "/usr/local/lib/python3.5/site-packages/flask_sqlalchemy/__init__.py", line 602, in __init__
    DeclarativeMeta.__init__(self, name, bases, d)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/ext/declarative/api.py", line 64, in __init__
    _as_declarative(cls, classname, cls.__dict__)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 88, in _as_declarative
    _MapperConfig.setup_mapping(cls, classname, dict_)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 103, in setup_mapping
    cfg_cls(cls_, classname, dict_)
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 133, in __init__
    self._setup_inheritance()
  File "/usr/local/lib/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 430, in _setup_inheritance
    "table-mapped class." % cls
sqlalchemy.exc.InvalidRequestError: Class <class 'flask_appbuilder.security.sqla.models.Permission'> does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class.

About the make_declarative reference, I don't have enough knowledge to properly understand what it is doing but I know that Flask-AppBuilder defines it this way https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/models/sqla/__init__.py#L23-L39

We had an issue with the number of argument, so we had to change it this same way in our project too: https://github.com/MichaelPereira/Flask-AppBuilder/pull/1/files#diff-d35cb966075d5560db04866a27e50fc8

Thanks again for your help and please let me know if I can be of more help

@davidism davidism added this to the 2.3 milestone Sep 13, 2017
@davidism
Copy link
Member

Closing this as it seems to be an issue with Flask-AppBuilder, not directly related to the tablename issue. Check #541.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants