Skip to content
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

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

Closed
QSHolzner opened this issue Oct 9, 2023 · 12 comments · Fixed by #816
Closed

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

QSHolzner opened this issue Oct 9, 2023 · 12 comments · Fixed by #816
Labels
documentation Improvements or additions to documentation

Comments

@QSHolzner
Copy link

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
Copy link
Author

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

@roman-right
Copy link
Member

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

@roman-right roman-right added the bug Something isn't working label Nov 5, 2023
@gsakkis
Copy link
Contributor

gsakkis commented Nov 6, 2023

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
Copy link
Member

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.

@roman-right roman-right removed the bug Something isn't working label Dec 4, 2023
@QSHolzner
Copy link
Author

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
Copy link
Member

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!

@roman-right roman-right added the documentation Improvements or additions to documentation label Dec 4, 2023
@gsakkis
Copy link
Contributor

gsakkis commented Dec 4, 2023

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
Copy link
Member

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

@gsakkis
Copy link
Contributor

gsakkis commented Dec 4, 2023

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

@roman-right
Copy link
Member

@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
Copy link
Contributor

gsakkis commented Dec 5, 2023

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants