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

Model with tablename of reflected table does not have table assigned #550

Closed
flipperbw opened this issue Oct 2, 2017 · 17 comments
Closed
Assignees
Milestone

Comments

@flipperbw
Copy link

flipperbw commented Oct 2, 2017

Just updated and my full application, working previously, broke down on setting the model due to the update (#541). I believe it might be a problem with this new primary key detection thing, which I'm not sure I get the point of. The previously-fine code is thus:

from flask import Flask
from flask_sqlalchemy import SQLAlchemy

app = Flask(__name__)
db = SQLAlchemy(app)
db.reflect()

class Country(db.Model):
    __tablename__ = 'd_country'

Resulting in:

InvalidRequestError: Class <class 'app.models.Country'> does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class.

I'd be willing to bet that the update didn't take db.reflect() into account. I'm rolling back unless there's something else I can do, but I'd rather not have to do a declared_attr on all my tables just for this thing.

@davidism
Copy link
Member

davidism commented Oct 2, 2017

You need to specify a primary key column for all models that do not inherit from another mapped model. This is true in normal SQLAlchemy too, the error messages are just switched due to some internal behavior.

@davidism davidism closed this as completed Oct 2, 2017
@davidism davidism reopened this Oct 2, 2017
@davidism
Copy link
Member

davidism commented Oct 2, 2017

Reopening because it would be good to fix which error gets raised. Will look into this.

@davidism davidism changed the title 2.3.0 broke __tablename__ loading Model without primary key column raises error about __tablename__ instead Oct 2, 2017
@flipperbw
Copy link
Author

It does have a primary key which gets loaded in during the reflect process. This worked fine before.

@davidism
Copy link
Member

davidism commented Oct 2, 2017

The code you've shown creates a model with no primary key and does not inherit from an existing mapped model. Make sure your example code reflects the problem you're describing.

@flipperbw
Copy link
Author

No...it does. Look:

$ ipy
In [1]: from flask import Flask
   ...: from flask_sqlalchemy import SQLAlchemy
   ...:
   ...: app = Flask(__name__)
   ...:
   ...: db = SQLAlchemy(app)
   ...: db.reflect()
   ...:
   ...: from app import models as m
In [2]: m.Country.__table__
Out[2]: Table('d_country', MetaData(bind=None), Column('code', CHAR(length=2), table=<d_country>, primary_key=True, nullable=False), Column('name', VARCHAR(length=99), table=<d_country>), schema=None)

This is in v2.2. 2.3 prevents me from loading with the exact same code.

@davidism
Copy link
Member

davidism commented Oct 2, 2017

Do you get any warnings from SQLAlchemy? In your real code, are you doing anything with that model besides setting a tablename?

@davidism
Copy link
Member

davidism commented Oct 2, 2017

I have a feeling this just worked by accident due to other bad behavior in 2.2, and you actually need to inherit from the reflected model and/or use extend_existing.

There were never any tests for this, nor the intention for this to work.

@flipperbw
Copy link
Author

The error is thrown at the time it is loaded with that exact code up there. Here's the full thing.

$ ipy
In [1]: from flask import Flask
   ...: from flask_sqlalchemy import SQLAlchemy
   ...:
   ...: app = Flask(__name__)
   ...:
   ...: db = SQLAlchemy(app)
   ...: db.reflect()
   ...:
   ...: from app import models as m
   ...:
---------------------------------------------------------------------------
InvalidRequestError                       Traceback (most recent call last)
<ipython-input-1-745089669a6c> in <module>()
     11 db.reflect()
     12
---> 13 from app import models as m

[snip]/app/__init__.py in <module>()
    153
    154
--> 155 from app import views

[snip]/app/views.py in <module>()
      1 from app import app, db
----> 2 from app.models import *
      3 from sqlalchemy import func

[snip]/app/models.py in <module>()
     16
     17
---> 18 class Country(db.Model):
     19     __tablename__ = 'd_country'
     20

/usr/local/lib/python2.7/dist-packages/flask_sqlalchemy/model.pyc in __init__(cls, name, bases, d)
     64             cls.__tablename__ = camel_to_snake_case(cls.__name__)
     65
---> 66         super(NameMetaMixin, cls).__init__(name, bases, d)
     67
     68     def __table_cls__(cls, *args, **kwargs):

/usr/local/lib/python2.7/dist-packages/flask_sqlalchemy/model.pyc in __init__(cls, name, bases, d)
     91         )
     92
---> 93         super(BindMetaMixin, cls).__init__(name, bases, d)
     94
     95         if bind_key is not None and hasattr(cls, '__table__'):

/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/api.pyc in __init__(cls, classname, bases, dict_)
     62     def __init__(cls, classname, bases, dict_):
     63         if '_decl_class_registry' not in cls.__dict__:
---> 64             _as_declarative(cls, classname, cls.__dict__)
     65         type.__init__(cls, classname, bases, dict_)
     66

/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.pyc in _as_declarative(cls, classname, dict_)
     86         return
     87
---> 88     _MapperConfig.setup_mapping(cls, classname, dict_)
     89
     90

/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.pyc in setup_mapping(cls, cls_, classname, dict_)
    101         else:
    102             cfg_cls = _MapperConfig
--> 103         cfg_cls(cls_, classname, dict_)
    104
    105     def __init__(self, cls_, classname, dict_):

/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.pyc in __init__(self, cls_, classname, dict_)
    131         self._setup_table()
    132
--> 133         self._setup_inheritance()
    134
    135         self._early_mapping()

/usr/local/lib/python2.7/dist-packages/sqlalchemy/ext/declarative/base.pyc in _setup_inheritance(self)
    428                 "Class %r does not have a __table__ or __tablename__ "
    429                 "specified and does not inherit from an existing "
--> 430                 "table-mapped class." % cls
    431             )
    432         elif self.inherits:

InvalidRequestError: Class <class 'app.models.Country'> does not have a __table__ or __tablename__ specified and does not inherit from an existing table-mapped class.

Besides the little snips for privacy, it's exactly the same code that just worked in 2.2. I don't have time to do anything with the model because I can't load it in.

@flipperbw
Copy link
Author

I believe I am inheriting properly by loading db from app, then using class X(db.Model). All joins and everything, which would rely on the keys, work properly.

@davidism
Copy link
Member

davidism commented Oct 2, 2017

You are not. db.Model is the base model class, not a real model. Your intention appears to be to get the reflected model, creating a new class with the same table name is not how you do that.

@flipperbw
Copy link
Author

So I guess I'm not sure why this worked with zero issues before in a fairly complex setup. Are you saying it was a bug of a bug that made it work? If so I feel like that behavior makes sense. What should I be doing instead besides that relatively straightforward-seeming approach?

@davidism
Copy link
Member

davidism commented Oct 2, 2017

Does this work in plain SQLAlchemy or Flask-SQLAlchemy 2.1?

@davidism davidism changed the title Model without primary key column raises error about __tablename__ instead Model with tablename of reflected table does not have table assigned Oct 2, 2017
@davidism
Copy link
Member

davidism commented Oct 2, 2017

OK, this does work in plain SQLAlchemy. Probably need to account for it in __table_cls__.

import sqlalchemy as sa
from sqlalchemy import orm
from sqlalchemy.ext.declarative import declarative_base

engine = sa.create_engine('postgresql:///example', echo=True)

Base = declarative_base()
Base.metadata.reflect(engine)

class User(Base):
    __tablename__ = 'user'

session = orm.Session(engine)
print(session.query(User).all())

@flipperbw
Copy link
Author

2.1 also works fine. For the sqlalchemy:

from sqlalchemy import create_engine, MetaData
from sqlalchemy.ext.declarative import declarative_base

engine = [xxxxx]
Base = declarative_base(bind=engine)
Base.metadata.reflect()

class X(Base):
    __tablename__ = 'd_country'

X.__mapper__.primary_key[0].name
Out[9]: 'code'

@flipperbw
Copy link
Author

Oh, you beat me to it. Are you saying I need to account for it or just that it does in F-SA?

@davidism davidism added this to the 2.3.1 milestone Oct 2, 2017
@davidism davidism self-assigned this Oct 2, 2017
@davidism
Copy link
Member

davidism commented Oct 2, 2017

Fixed in #551. Will be released soon, want to look into one other issue first.

@cowgill
Copy link

cowgill commented Mar 21, 2019

You need to specify a primary key column for all models that do not inherit from another mapped model. This is true in normal SQLAlchemy too, the error messages are just switched due to some internal behavior.

I know this is an old issue, but I've searched high and low trying to find a flask-sqlalchemy reflect example that actually works.

Could you point me to a working example (using an app factory)? Much appreciated.

@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

3 participants