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

Refactor related #8

Merged
merged 9 commits into from Jun 29, 2015

Conversation

@jmcarp
Copy link
Contributor

commented Jun 21, 2015

Model related rows with Related field.

The current implementation of relationship fields based on QuerySelect
and QuerySelectList fields makes many potentially expensive queries.
This patch adds a SQLAlchemy-specific field, Related, and uses it for
relationships.

Note: with the move away from QuerySelect, we lose the keygetter option.
I'm not sure I liked this interface to begin with (Is it a good idea for all related
fields to use the same keygetter? If keygetter can return some computed
property of a row, how does the user specify a way to load data from the ORM
for efficient deserialization?). If we want to restore functionality like serializing
related URLs instead of primary keys, I think we should subclass the proposed
Related field to HyperlinkRelated, SlugRelated, etc., as in DRF. Thoughts
@sloria?

[Resolves #7]

Model related rows with `Related` field.
The current implementation of relationship fields based on `QuerySelect`
and `QuerySelectList` fields makes many potentially expensive queries.
This patch adds a SQLAlchemy-specific field, `Related`, and uses it for
relationships.

[Resolves #7]

@jmcarp jmcarp force-pushed the jmcarp:refactor-related branch from f29a906 to 4608bf1 Jun 21, 2015

@jmcarp jmcarp force-pushed the jmcarp:refactor-related branch from 4608bf1 to 7f36ba3 Jun 21, 2015

jmcarp added some commits Jun 27, 2015

@jmcarp jmcarp referenced this pull request Jun 27, 2015
@jmcarp jmcarp referenced this pull request Jun 28, 2015
Fix `related_model` and updates tests.
h/t @sloria for the issue and the fix

@jmcarp jmcarp force-pushed the jmcarp:refactor-related branch from 3bf241c to 38acda8 Jun 28, 2015

@jmcarp jmcarp force-pushed the jmcarp:refactor-related branch from 203979a to 670d098 Jun 28, 2015

@sloria sloria merged commit f13e30f into marshmallow-code:dev Jun 29, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.