BeanieODM / beanie

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

[BUG] AWS DocumentDB does not work with 1.14.0 - Not found for _id: ... #402

Closed mickdewald closed 2 years ago

mickdewald commented 2 years ago

Describe the bug I noticed, that since I updated to beanie 1.14.0, my program does not work with AWS DocumentDB anymore. This has not been a problem prior, and the same code works perfectly with 1.13.1

Additionally, the code works perfectly fine with 1.14.0 against the local MongoDB test database in version 5.0.10.

The error message is not very helpful, the requested resources can simply be not found (although they are there)

NotFound '<some_OID>' for '<class 'mongodb.model.user.odm.User'>' 
not found in database 'User' with id '<some_OID>' not found in database

To verify, that the resource is there I use a tool like NoSQLBooster or Robo3T

db.user.find( {"_id" : ObjectId("<some_OID>")}  )
   .projection({})
   .sort({_id:-1})
   .limit(100)

To Reproduce

# Nothing special, just a simple find command
result = await model.find_one(model.id == oid)

Expected behavior I expected beanie 1.14.0 to work with AWS DocumentDB the same way as 1.13.1

Additional context I am glad to provide further information, or I can make some tests against DocumentDB if someone can give me hints what to do.

roman-right commented 2 years ago

Hi, Most likely you use Collection inner class to set up the collection name. Since 1.14 it is not supported more. Please, use Settings inner class instead.

mickdewald commented 2 years ago

Firstly, thanks for answering so fast, I really appreciate it!

So, if I understand you correctly, I should change the following code

class Stock(Document):
    symbol: str

    class Collection:
        name = "stock"  # <-- This is the problem with 1.14.0?

    class Settings:
        use_state_management = True 

to something like this

class Stock(Document):
    symbol: str

#    class Collection:
#        name = "stock"  # <-- Deprecated

    class Settings:
        name = "stock"  # <-- Is this the correct way with 1.14.0?
        use_state_management = True

I will give it a try.

But why does it work with MongoDB (in my local test database) and not with DocumentDB?

mickdewald commented 2 years ago

So, I changed my code according to your recommendation and used the Settings class as proposed as:

class Stock(Document):
    symbol: str

    class Settings:
        name = "stock"
        use_state_management = True

I can confirm, that it works now with v1.15.0 and DocumentDB.

I greatly appreciate your help! And thanks for this wonderful library!

roman-right commented 2 years ago

Nice! It was working for the local db bc you inserted documents there using the same config (which was ignored and used the default collection name instead). While in the DocumentDB you already created docs in the collection with the specified name.

mickdewald commented 2 years ago

It was working for the local db bc you inserted documents there using the same config (which was ignored and used the default collection name instead). While in the DocumentDB you already created docs in the collection with the specified name.

Hm, ok, I see. This would work in the test scenario, when the database was freshly built, so the table name would be "Stock" (capital S) instead of (stock). Is that what you mean?

That might explain the CI/CD build success with the local db. But it also worked with my local dev db, which has already been created before (including tables and data) with the old settings (that means lower case letters for tables like "stock").

I mean, I do not think it is necessary to put much thought in here right now because it works now with the new and recommended settings. It's just my observation, that might be interesting for future matters.

roman-right commented 2 years ago

Hm, then it is weird. Anyway, I'll add the warning message to the next few versions in case the Collection inner class was used to be sure I did everything I could :)

mickdewald commented 2 years ago

Hm, then it is weird. Anyway, I'll add the warning message to the next few versions in case the Collection inner class was used to be sure I did everything I could :)

Awesome! I mean, ultimately, it was my fault not reading the release notes. And having just written

# requirements.txt
beanie

without version (and thus automatically fetch the latest version in CI/CD) was also not helpful in my case ;)