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

Missing TimeZone info from deserialized value #396

Closed
Shriyanshagro opened this issue Aug 23, 2017 · 6 comments
Closed

Missing TimeZone info from deserialized value #396

Shriyanshagro opened this issue Aug 23, 2017 · 6 comments
Assignees

Comments

@Shriyanshagro
Copy link
Member

Shriyanshagro commented Aug 23, 2017

Restapi removes the TimeZone info from datetime object when deserialized.
Which eventually raises the TypeError or MissMatchError when compared(with the original value).

A Simple failing example is described below with Breakdown of desearialize code

Case I: Value doesn't have any specified TimeZone info
(Pdb) value = '2017-08-15T19:00:00'
(Pdb) DateTime(value)
DateTime('2017/08/15 19:00:00 GMT+0')
(Pdb) DateTime(value).toZone(DateTime().localZone()).asdatetime().replace(tzinfo=None)
datetime.datetime(2017, 8, 15, 19, 0)
Hurrah!! everything good here

Case II: Value has specified TimeZone info
(Pdb) value = '2017-08-15T19:00:00+05:30'
(Pdb) DateTime(value).asdatetime()
datetime.datetime(2017, 8, 15, 19, 0, tzinfo=<StaticTzInfo 'GMT+0530'>)
(Pdb) DateTime(value).toZone(DateTime().localZone()).asdatetime()
datetime.datetime(2017, 8, 15, 13, 30, tzinfo='UTC')
(Pdb) DateTime(value).toZone(DateTime().localZone()).asdatetime().replace(tzinfo=None)
datetime.datetime(2017, 8, 15, 13, 30)
Notice the MissMatchError of value after desearialized, while my my localZone is still +05:30

vm is datetime object of value = '2017-08-15T19:00:00+05:30'
vm = datetime.datetime(2017, 8, 15, 19, 0, tzinfo=pytz.timezone('Asia/Kolkata'))

Case III: if we compare dv(desearialized value) from Case II to vm
if dv == vm:
...: print 'solved'
TypeError: can't compare offset-naive and offset-aware datetimes

Case IV: If we don't aggressively remove tzinfo from Case II?
(Pdb) v = DateTime(value).toZone(DateTime().localZone()).asdatetime()
datetime.datetime(2017, 8, 15, 13, 30, tzinfo='UTC')
if v != vm:
...: raise MissMatchError
MissMatchError: Due to conversion of timezone to localZone(), no matter what was the original timezone value

This issue is in continuation to #377 which was raised in plone.importexport module

@Shriyanshagro Shriyanshagro changed the title Missing TimeZone info from desearialized value Missing TimeZone info from deserialized value Aug 25, 2017
@loechel
Copy link
Member

loechel commented Aug 28, 2017

Ok, steps to reproduce:

You need an event object for example: /Plone/events/test-event

curl -v -i "http://plone.site/Plone/events/test-event" -H "Accept: application/json" --user admin:secret

will return a json object it normaly did contain a start, maybe an end datetime object

if you now try to update this field it raises an TypeError:

curl -v -i -X PATCH "http://plone.site/Plone/events/test-event" -H "Accept: application/json" --user admin:secret -H "Content-Type: application/json" --data-raw '{"start": "2017-08-29T00:00:00+02:00"}'

Result:

TypeError: can't compare offset-naive and offset-aware datetimes

which is raised here:

@loechel loechel self-assigned this Aug 28, 2017
@loechel loechel mentioned this issue Aug 28, 2017
@tisto
Copy link
Sponsor Member

tisto commented Aug 29, 2017

@loechel thank you for the detailed bugreport. :)

@sneridagh
Copy link
Member

sneridagh commented Aug 29, 2017

In fact, it's the same issue I fixed in this issue #253 and fixed in #394.

@tisto
Copy link
Sponsor Member

tisto commented Aug 29, 2017

@loechel can you please confirm and close the issue if it has been solved properly?

@loechel
Copy link
Member

loechel commented Aug 29, 2017

@tisto I can confirm that it is the same issue, and that @sneridagh pull request #394 would fix, but that pull request is still open.

Looking at that #394 pull request I would suggest one change, that I will either do as a review or a commit to this branch if @sneridagh allows.

@loechel
Copy link
Member

loechel commented Jan 25, 2018

should also be fixed with the superseeding #415 for #394.

If @Shriyanshagro still see an issue, please open a new ticket.

@loechel loechel closed this as completed Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants