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

Add serialization with timezone to AwareDateTime #1676

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

theirix
Copy link

@theirix theirix commented Oct 9, 2020

Allows AwareDateTime field to optionally serialize naive datetime with default timezone.
Behaviour is optional and enabled with use_serialization=True in ctor.

For example, if date-time is stored in a database without a timezone and we want to treat it as UTC datetime, new serializer helps a lot.

Allows AwareDateTime field to optionally serialize naive datetime
with default timezone.
@odesenfans
Copy link

I was just looking for this change! I'm wondering: for some use cases, it makes sense IMO to localize aware datetimes as well (your fix just converts naive datetimes and leaves aware datetimes untouched). For example, using PostgreSQL + SQLAlchemy, datetimes are retrieved in the timezone of the DB connection (UTC by default).

It would be cool to ignore this implementation detail by just specifying my preferred output timezone in the schema and be done with it. What do you think?

@theirix
Copy link
Author

theirix commented Nov 29, 2020

Sorry, missed your comment. I think, your use case is better handled by creating a third DateTime descendant that stores a timezone passed in ctor and forces conversion an aware datetime (from DB) to this timezone for both serialization and deserialization. NaiveDateTime and AwareDateTime are a sort of mutually exclusive in handling awareness so it is not clear which class would handle proposed logic.

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

Successfully merging this pull request may close these issues.

2 participants