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

DataSet thaw() exports dates in isoformat() which is not on the format list for DateTimeField #1536

Closed
yevrah opened this issue Mar 13, 2018 · 4 comments

Comments

@yevrah
Copy link

yevrah commented Mar 13, 2018

We are using a script to freeze copies of our staging and prod MySQL database. For development we are using SQLite, using the thawed data from MySQL. However, this issue can be replicated just from freezing and thawing between SQLite entities.

Test script below to show the issue.

# test.py
from peewee import *
from datetime import datetime
from playhouse.dataset import DataSet
from os import remove

# Reset
try: remove('user.db')
except: pass

# Db
db = SqliteDatabase('user.db')

class User(Model):
    name = CharField(default='John Doe')
    last_login = DateTimeField()

    class Meta:
        database = db

class UserThawed(User):
    pass


if __name__ == "__main__":

    # Create entity
    db.create_tables([User])
    User(name='John Doe', last_login=datetime.now()).save()

    # Blind Test
    john = User().get()
    print("\n\nOriginal Entity\n=====================")
    print("Last Login: {}".format(john.last_login))
    print("Type of:    {}".format(type(john.last_login)))

    # Freeze
    ds = DataSet('sqlite:///user.db')
    user = ds['user']
    ds.freeze(user.all(), format='json', filename='user.json')

    # Thaw
    user_thawed = ds['userthawed']
    user_thawed.thaw(format='json', filename='user.json')

    # Blind Test for Thawed Item
    john_thawed = UserThawed().get()
    print("\n\nThawed Entity\n=====================")
    print("Last Login: {}".format(john_thawed.last_login))
    print("Type of:    {}".format(type(john_thawed.last_login)))

Which outputs the following when run.

# Output from > python test.py

Original Entity
=====================
Last Login: 2018-03-13 12:06:29.403321
Type of:    <class 'datetime.datetime'>


Thawed Entity
=====================
Last Login: 2018-03-13T12:06:29.403321
Type of:    <class 'str'>

I noticed that freeze uses datetime.isoformat() to convert DateTimeField, and DateField - but this format is not in the format lists.

We have tested a quick fix by adding DateTimeField.formats.append('%Y-%m-%dT%H:%M:%S'), and DateField.formats.append(''%Y-%m-%dT%H:%M:%S'') to our models file - which resolved the MySQL to SqlLite issue.

From our testing between SQLite to SQLite, it would seem '%Y-%m-%dT%H:%M:%S.%f' would also need to be added to the list.

It would be great to have it added to the format lists at https://github.com/coleifer/peewee/blob/master/peewee.py#L3955 and https://github.com/coleifer/peewee/blob/master/peewee.py#L3974

Have not tested TimeField, and I haven't been able to dig around the code enough to understand if appending it to sqlite_datetime_formats, and sqlite_date_trunc is necessary.

@coleifer
Copy link
Owner

This is a bit tricky because JSON should utilize ISO format, but Peewee really only has date-parsing logic to work-around the fact that SQLite doesn't have a native datetime data-type. By convention, Peewee has used the equivalent of Python's __str__() for datetime/date/time objects when persisting in SQLite.

Adding more formats is certainly one route, but my preference is to just change the export format to match what peewee already can handle. Fewer changes. If you want ISO, you can always subclass and extend DataSet.

@coleifer
Copy link
Owner

b65a8cb -- str() instead of isoformat, includes test.

@yevrah
Copy link
Author

yevrah commented Mar 13, 2018

Thanks @coleifer makes much more sense. Didn't occur to me to fix it at the root, thought ISO made sense as it's also what I normally use. Cheers, much appreciated.

@coleifer
Copy link
Owner

ISO made sense

Well that was the problem, it does make sense. But it seemed expedient to move to a serialization format that could inter-operate with Peewee as-is and leave implementing an ISO date format for a separate Exporter class.

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

No branches or pull requests

2 participants