klen / peewee-aio

Async support for Peewee ORM
44 stars 7 forks source link

Retrieving a nested model asynchronously is unnecessary #13

Open F1int0m opened 4 months ago

F1int0m commented 4 months ago

I have these models:

@manager.register
class Role(AIOModel):
    id = fields.AutoField()
    name = fields.CharField()

@manager.register
class User(AIOModel):
    id = fields.AutoField()
    name = fields.CharField()
    role = fields.ForeignKeyField(Role)

I want to query the database and get a list of models and then use the nested model somehow. I want to make a request like this:

User.select(User, Role)

The code should now look like this:

async for user in result:
    role = await user.role
    print(f'user {user.id} have role {role.id}')

This code looks ugly and inconvenient. In addition, receiving model Role from the user object is internally synchronous (because peewee has already enriched the main model with a nested model).

This is because AIOForeignKeyField overrides accessor_class so that the .get_rel_instance() function becomes asynchronous.

But there is no need for this in my request. All information about the nested model is already in instance.__rel__ and is given synchronously.

I did this crutch for myself:

import peewee
from peewee-aio import fields

class MetaBaseModel(AIOModelBase):
    def __new__(cls, name, bases, attrs):
        """
        Replaces a function by removing implicit and uncontrolled field autocorrects.
        """

        cls = super(AIOModelBase, cls).__new__(cls, name, bases, attrs)
        meta = cls._meta
        if getattr(cls, '_manager', None) and not meta.database:
            meta.database = cls._manager.pw_database

        return cls

class BaseModel(AIOModel, metaclass=MetaBaseModel):
    pass

@manager.register
class Role(BaseModel):
    id = fields.AutoField()
    name = fields.CharField()

@manager.register
class User(BaseModel):
    id = fields.AutoField()
    name = fields.CharField()
    role = peewee.ForeignKeyField(Role)

In this case, I can access the nested model directly without "await":

result = User.select(User, Role)
async for user in result:
    print(f'user {user.id} have role {user.role.id}')

Proposed solution

Make field overriding optional through some parameter in this function:

# model.py
 class BaseConfig:
    auto_correct_fields = True

class AIOModelBase(ModelBase):
    ...
    config = BaseConfig

    def __new__(cls, name, bases, attrs):
        ...
        for attr_name, attr in attrs.items():
            if not isinstance(attr, Field) or isinstance(
                attr, (AIOForeignKeyField, AIODeferredForeignKey, FetchForeignKey)
            ) or getattr(cls.Config, 'auto_correct_fields', False):
                continue

Then I can choose for myself when the fields need to be redefined, and when I want to use the original peewee fields:

@manager.register
class User(AIOModel):
   ...

    class Config:
        auto_correct_fields = True
klen commented 4 months ago

Foreign keys are awaitable because you may load them on demand (like peewee does):

for user in await User.select():
    role = await user.role # load the role (yes the example is not optimised)
    print(f'user {user.id} have role {role.name}')

In case you had preloaded related models, like in your example before await user.role wont make a request to DB. But if you want to do it synchronously there is a method in peewee-aio:

for user in await User.select(User, Role).join(Role, src=User):
    role = User.role.fetch(user) # the role has to be preloaded
    print(f'user {user.id} have role {role.name}')
F1int0m commented 4 months ago

Your code example does exactly what I want.

But in fact, I don’t need to work directly with the nested model, but I need playhouse.shortcuts.model_to_dict() to work correctly.

Now it crashes with the following errors:

  File "...", line 84, in model_to_dict
    field_data = model_to_dict(
  File "...", line 75, in model_to_dict
    for field in model._meta.sorted_fields:
AttributeError: 'coroutine' object has no attribute '_meta'
sys:1: RuntimeWarning: coroutine 'AIOForeignKeyAccessor.get_rel_instance' was never awaited

That's why I wanted to change the main field so that the internals of peewee work correctly not only at the level of my application, but also in other functions from the playhouse.

klen commented 4 months ago

That's because playhouse.shortcuts.model_to_dict() is synchronous function (it makes sync DB queries). I think it should be rewrited as async, to support async nature of peewee-aio.