BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
2.01k stars 212 forks source link

[Question] What's the best way to dynamically call Motor.io functions? #13

Closed RodneyU215 closed 3 years ago

RodneyU215 commented 3 years ago

First and foremost thank you for creating such a useful tool! It's exactly what I've been looking for.

Now for context I was looking to use a motor method count_documents() that was not exposed at the Document level. I noticed in the documentation that it's suggested to use Document.get_motor_collection().count_documents(). This indeed worked but its not very clean imo. I could also explicitly redefine in my User class any methods I wanted use from motor that weren't defined in Document. But this too felt like extra code I shouldn't have to write. So instead I created this metaclass to dynamically call them.

What are your thoughts about this? Is there a better way? Are there any gotchas I should be concerned with? If you believe this change would actually be helpful to others would you like me to submit a PR?

from beanie import Document
from motor.motor_asyncio import AsyncIOMotorCollection # Note: I had to use AsyncIOMotorCollection because I ran into recursive issues due to other areas in the codebase where you call "getattr".
from pydantic.main import ModelMetaclass # Note: Pydantic's BaseModel has a custom metaclass; therefore to use a metaclass of our own we have to extend theirs.

class DocumentMetaClass(ModelMetaclass):
    def __getattr__(cls, attr):
        if callable(getattr(AsyncIOMotorCollection, attr, None)): # Note: I only want to execute this code if the attr that's missing exists in motor.
            motor_collection = cls.get_motor_collection()
            motor_func = classmethod(getattr(motor_collection, attr))
            setattr(cls, attr, motor_func)
            return getattr(cls, attr)

class User(Document, metaclass=DocumentMetaClass): # Note: This assignment of metaclass could be done at the Document level if it's helpful to others.
    username: str
    email: str

# skipping async code...etc

await User.count_documents({ 'username': user.username }, limit = 1)
roman-right commented 3 years ago

Hi Rodney,

Thank you for the feedback!

Yes, it looks good. Interesting feature.

Unfortunately, I see some potential problems with this approach. Right now Document class has some methods with the same names, as the motor collection has, but with different interfaces and different outputs. And later I plan to add more (like count_documents() from your example will appear in 4.0.0b1 soon. The problem is, in that case, it will override the dynamically attached method from the motor collection. This collision can be a problem if somebody already uses this motor method directly. Also, Document itself is inherited from BaseModel. Methods could be added and on the Pydantic side and on the Motor side then - the behavior is out of control in this case. I think the best strategy here would be to implement popular methods in Document one by one with reasonable changes and keep others inside the motor collection.

What do you think about this? Does it make sense?

RodneyU215 commented 3 years ago

I hear what you're saying and you're right to be cautious!

To clarify: __getattr__ is only called when the method is unable to be located on your Document class, which like you mentioned includes BaseModel. So the order in which Python would look for an existing method on class User(Document) would be..

  1. Defined on my User?
  2. Defined on Document?
  3. Defined on BaseModel?
  4. Defined on AsyncIOMotorCollection? The first that matches gets called.

The part I do agree with is that it's a bit "magical". I'm just not sure I see or understand yet the value of recreating the Motor methods. I'll take a look at that beta branch.

roman-right commented 3 years ago

Yes, you are right. I'm talking about the case, when I use the method from AsyncIOMotorCollection and in the next version a method with the same name appears in Document or even in BaseModel, but with different behavior. I'll face an error or wrong logic after the update. It is impossible to mark this as deprecated because it is out of control.

RodneyU215 commented 3 years ago

I see what you mean now! You're right folks would be forced to update their code to reflect any Motor's of changes. That could end up being a pretty bad dev experience if they don't know where the method was changed.

Okay that answers my question. I'll use this with caution. 😅 Thanks again!

RodneyU215 commented 3 years ago

I'm leaving this here incase anyone else stumbles across it. I agree with the concerns that Roman has raised. I personally still wanted another way to quickly call Motor functions while taking Roman's feedback into consideration. I ended up with this:

from pydantic.main import ModelMetaclass
from motor.motor_asyncio import AsyncIOMotorCollection
from beanie import Document

class DocumentMetaClass(ModelMetaclass):
    def __getattr__(cls, attr):
        if attr.startswith('db_'):
            motor_method = attr.split("db_")[1]
            if callable(getattr(AsyncIOMotorCollection, motor_method, None)):
                motor_collection = cls.get_motor_collection()
                motor_func = classmethod(getattr(motor_collection, motor_method))
                setattr(cls, attr, motor_func)
                return getattr(cls, attr)
        else:
            raise AttributeError(f'{cls.__name__}.{attr}() cannot be found. To call a AsyncIOMotorCollection function try {cls.__name__}.db_{attr}() instead!')

class BaseDocument(Document, metaclass=DocumentMetaClass):
    pass

class User(BaseDocument):
    pass

By extending my BaseDocument class I'm able to explicitly call motor functions when they do not exist yet within Beanie.

$ User.count_documents({ 'username': username }, limit = 1)
~> AttributeError: User.count_documents() cannot be found. To call a AsyncIOMotorCollection function try User.db_count_documents() instead!
$ User.db_count_documents({ 'username': username }, limit = 1)
~> 1

I personally love Beanie for what it does. I like that it's a "micro" ODM. Therefore, I personally only want to rely on it for the components that are absolutely necessary. Everything else I'd like to default back to motor to ensure I can always use whatever is available from MongoDB. This solution helps me with that.