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

Don't assume id for primary key column #111

Closed
jace opened this issue Jun 5, 2017 · 2 comments
Closed

Don't assume id for primary key column #111

jace opened this issue Jun 5, 2017 · 2 comments

Comments

@jace
Copy link
Member

jace commented Jun 5, 2017

Several places in HasGeek code including coaster and baseframe refer to self.id or obj.id to retrieve the primary key. This is incorrect usage. The correct method per SQLAlchemy is the following:

from sqlalchemy import inspect
identity = inspect(obj).identity

Note that the identity value here is a tuple and not a scalar as assumed by obj.id.

@jace
Copy link
Member Author

jace commented Jun 5, 2017

Coaster's use of self.id is currently justified because all usage is within derivatives of IdMixin, which explicitly defines the id column. However, Baseframe's ticket remains valid.

@jace jace closed this as completed Jun 5, 2017
@jace jace mentioned this issue Jun 5, 2017
2 tasks
@jace
Copy link
Member Author

jace commented Jun 6, 2017

Use of self.id has been removed in #112 and #113.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant