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

about the region get_multi / get_or_create_multi return type #35

Closed
sqlalchemy-bot opened this Issue Jun 15, 2013 · 4 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Jun 15, 2013

Migrated issue, originally created by lxyu (lxyu)

Currently the get multi apis returns list, while I think the dict is a better choice.

Take this code for example:

If return list, I have to make sure all order_ids exist, also must I sort the result based on the keys. If any key misses or the order of result mixed, it can lead to incorrect cache, which is severe error.

#!python
@region.cache_multi_on_arguments()
def get(*order_ids):
    orders = DBSession().query(Order).filter(Order.id.in_(order_ids)).all()
    assert(len(orders) == len(order_ids))
    return sorted(orders, key=lambda o: order_ids.index(o.id))

If return dict, it'll be much simpler:

#!python
@region.cache_multi_on_arguments()
def get(*order_ids):
    orders = DBSession().query(Order).filter(Order.id.in_(order_ids))
    return {order.id: order for order in orders}

So if the return type is dict, it can be more fault tolerant without lose in function.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 15, 2013

Michael Bayer (zzzeek) wrote:

are we talking about the get_multi() return type (this returns NO_VALUE for missing keys so the result is unambiguous, just call dict(zip(keys, values)), or the data type expected by the decorated function in cache_multi_on_arguments() (in which case, add a keyword argument to cache_multi_on_arguments(asdict=True) ), there's no issue having both ways for that one?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 15, 2013

lxyu (lxyu) wrote:

yep, asdict=True would be great!

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 15, 2013

Michael Bayer (zzzeek) wrote:

4d28426

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jun 15, 2013

Changes by Michael Bayer (zzzeek):

  • changed status to closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment