BeanieODM / beanie

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

[BUG] Error when saving Documents with Fields of type date #736

Closed QSHolzner closed 8 months ago

QSHolzner commented 11 months ago

Describe the bug

If the type of a field is date and not datetime, then the following error occurs when saving:

Traceback (most recent call last):
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 242, in _encode
    data = dict(obj)
           ^^^^^^^^^
TypeError: 'datetime.date' object is not iterable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 246, in _encode
    data = vars(obj)
           ^^^^^^^^^
TypeError: vars() argument must have __dict__ attribute

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/workspaces/demo/scripts/test.py", line 74, in <module>
    asyncio.run(main())
  File "/usr/local/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/workspaces/demo/scripts/test.py", line 70, in main
    await pm.create()
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/documents.py", line 339, in create
    return await self.insert(session=session)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/actions.py", line 219, in wrapper
    result = await f(self, *args, skip_actions=skip_actions, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/state.py", line 66, in wrapper
    result = await f(self, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/state.py", line 76, in wrapper
    result = await f(self, *args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/self_validation.py", line 12, in wrapper
    return await f(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/documents.py", line 315, in insert
    get_dict(
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/dump.py", line 23, in get_dict
    ).encode(document)
      ^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 91, in encode
    return self._encode(obj=obj)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 215, in _encode
    return self.encode_document(obj)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 163, in encode_document
    obj_dict[k] = encoder.encode(obj_dict[k])
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 91, in encode
    return self._encode(obj=obj)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/demo/.venv/lib/python3.11/site-packages/beanie/odm/utils/encoder.py", line 249, in _encode
    raise ValueError(errors)
ValueError: [TypeError("'datetime.date' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')]

To Reproduce

import asyncio
import datetime
from beanie import Document, init_beanie
from motor.motor_asyncio import (
    AsyncIOMotorClient,
)
class ProjectModel(Document):
    name: str
    start_date: datetime.date

    class Settings:
        name = "Projects"

async def init_db() -> None:
    client = AsyncIOMotorClient(DB_SERVER, directConnection=True, tz_aware=True)
    await init_beanie(database=client["demo"], document_models=[ProjectModel])

async def main():
    await init_db()
    pm = ProjectModel(
        name="T1",
        start_date=datetime.date.today(),
    )
    await pm.create()

if __name__ == "__main__":
    asyncio.run(main())

If the type of start_date is changed to datetime.datetime it works.

Expected behavior A Field of type date should be saved in the db as datetime.

Additional context The dictionary ENCODERS_BY_TYPE contains special conditions for certain types.

Would it be possible to include the type date here? In PyMongo only the type datetime is accepted, so it makes sense to convert the date fields in the model to datetime so that they can be written to the DB.

Here is an example of how I extend the dictionry with the special treatment for this type. It would be nice if this is provided by default.

import datetime
from beanie.odm.utils.encoder import ENCODERS_BY_TYPE

def _date_to_datetime(val):
    if val is None:
        return None
    return datetime.datetime(val.year, val.month, val.day)

def extend_encoders():
    ENCODERS_BY_TYPE[datetime.date] = _date_to_datetime
QSHolzner commented 10 months ago

The Dictionary ENCODERS_BY_TYPE was renamed to DEFAULT_CUSTOM_ENCODERS in the newest beanie version (1.23.0).

roman-right commented 10 months ago

Hi, Sorry for the late reply. I'll check it and fix during the next bug-fixing session

gsakkis commented 10 months ago

Is this the suggested way to store dates in MongoDB? AFAICT there's no dedicated date type; confusingly there is BSON Date type but that's the equivalent of Python's datetime, not date.

roman-right commented 9 months ago

Hi @QSHolzner , There are many ways people represent date in the database. It can be a timestamp object with zeroes in hours, minutes and etc, it can be a date string and etc. So, there is no standard there. Additionally there is type conflict if I add date type to the encoder (it conflicts with the binary type if I remember correctly). I'd suggest using custom json encoder or not using date time at all, as pymongo maps bson.date to the datetime type.

QSHolzner commented 9 months ago

Hi @roman-right, Thank you for your answer. I understand that there are different approaches how datetime values are stored in MongoDB. As @gsakkis already wrote there is a date type in BSON which is mapped as datetime in Python. This is because the date type in BSON always has a time part. We have decided to save a python datetime.date value as a BSON date with the time element 0. There are certainly different approaches to this problem and it is therefore understandable if an implementation like the one I suggested in beanie specifies too much. For me that's okay, we've already found a solution for our case.

But if beanie is used "out of the box", then the use of datetime.date in a Document field currently leads to a cryptic error (ValueError: [TypeError("'datetime.date' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')]). Perhaps it should be pointed out in the documentation that the datetime.date type is not supported without additional adjustments or a more explicit error message should be issued at the appropriate point.

roman-right commented 9 months ago

Hi @QSHolzner , This is a very good point. I will revisit this today and check if I can use default behavior for date there. If no, I'll add this to the doc. Thank you!

gsakkis commented 9 months ago

But if beanie is used "out of the box", then the use of datetime.date in a Document field currently leads to a cryptic error (ValueError: [TypeError("'datetime.date' object is not iterable"), TypeError('vars() argument must have dict attribute')]).

Agreed; for me this whole section that calls dict() and vars() on an unknown object is a risky hack that should be removed and replaced with raising a clear ValueError message.

roman-right commented 9 months ago

Hi, @gsakkis. Would you mind to pick this one up?

gsakkis commented 9 months ago

Hi @roman-right, you mean the removal of the code I linked to?

roman-right commented 9 months ago

@gsakkis this part or the entire problem with the date type. Both are placed in the same module which you know well. As I understood you have some ideas about the implementation. If you have time for this, for sure.

gsakkis commented 9 months ago

My understanding based on the discussion above was that Beanie should not include an encoder for date as there is no standard. Or are you suggesting to use the one @QSHolzner used (BSON date with the time element 0)?

roman-right commented 9 months ago

I don't have strong opinion about this specific type. I mean, we can have a default representation for sure. The most important thing here is that the exception is not very intuitive. If not incorrect even.