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

load_models should support joined loads #78

Closed
jace opened this issue Dec 2, 2015 · 2 comments
Closed

load_models should support joined loads #78

jace opened this issue Dec 2, 2015 · 2 comments

Comments

@jace
Copy link
Member

jace commented Dec 2, 2015

A typical load_models decoration looks like this:

@load_models(
    (Profile, {'name': 'profile'}, 'g.profile'),
    (ProposalSpace, {'name': 'space', 'profile': 'profile'}, 'space'),
    (TicketType, {'name': 'name', 'proposal_space': 'space'}, 'ticket_type'),
    permission='event-edit')

It asks for instances of Profile, ProposalSpace and TicketType to be loaded in sequence, raising a 404 if one is not found. This requires three database roundtrips, which is inefficient. This can be done in one query if we work up from the bottom:

TicketType.query.join(ProposalSpace).join(Profile).filter(TicketType.name == name, ProposalSpace.name == space, Profile.name == g.profile).first_or_404()

To support this syntax, load_models could change the tuple format it accepts slightly. The second item in the tuple is a mapping of a model attribute to a view parameter, for use with the filter_by filter. This could instead be a column (a SQLAlchemy Instrumented Attribute) that is applied with a filter filter. Since the column-containing models are also required to perform a join, the wrapping tuple could accept a fourth item, a model or a tuple of models that must be joined (L-to-R) (or this could be guessed by looking up the models to which the given filter columns are attached).

@jace
Copy link
Member Author

jace commented Dec 2, 2015

Caveat: This will break redirect handlers in the chain like this example:

@load_models(
    (Profile, {'name': 'profile'}, 'g.profile'),
    ((ProposalSpace, ProposalSpaceRedirect), {'name': 'space', 'profile': 'profile'}, 'space'),
    (TicketType, {'name': 'name', 'proposal_space': 'space'}, 'ticket_type'),
    permission='event-edit')

@jace
Copy link
Member Author

jace commented Feb 27, 2018

Joined loads are supported via InstanceLoader introduced in #167. This ticket is now irrelevant. The redirect handler issue can be solved by writing a custom loader wherever it is needed.

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

No branches or pull requests

1 participant