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.

Sign up for GitHub

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

Jump to bottom

fix: python datetime [de]serialization bug #734

Merged
merged 1 commit into from
Jul 2, 2021
Merged

fix: python datetime [de]serialization bug #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion python/looker_sdk/rtl/serialize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def unstructure_hook(api_model):
return data


DATETIME_FMT = "%Y-%m-%dT%H:%M:%S.%f%z"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the infinite syntax variation for datetime string formatting

if sys.version_info < (3, 7):
from dateutil import parser

Expand All @@ -158,9 +159,10 @@ def datetime_structure_hook(
def datetime_structure_hook(
d: str, t: Type[datetime.datetime]
) -> datetime.datetime:
return datetime.datetime.strptime(d, "%Y-%m-%dT%H:%M:%S.%f%z")
return datetime.datetime.strptime(d, DATETIME_FMT)


converter31.register_structure_hook(datetime.datetime, datetime_structure_hook)
converter40.register_structure_hook(datetime.datetime, datetime_structure_hook)
cattr.register_unstructure_hook(model.Model, unstructure_hook) # type: ignore
cattr.register_unstructure_hook(datetime.datetime, lambda dt: dt.strftime(DATETIME_FMT))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the whole fix

33 changes: 25 additions & 8 deletions python/tests/rtl/test_serialize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
# THE SOFTWARE.

import copy
import datetime
import enum
import functools
import json
Expand All @@ -42,8 +43,7 @@


class Enum1(enum.Enum):
"""Predifined enum, used as ForwardRef.
"""
"""Predifined enum, used as ForwardRef."""

entry1 = "entry1"
entry2 = "entry2"
Expand Down Expand Up @@ -99,6 +99,7 @@ class Model(ml.Model):
# standard types
id: Optional[int] = None
name: Optional[str] = None
datetime_field: Optional[datetime.datetime] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding this datetime_field everywhere in the tests identically reproduced the issue in #720


# testing reserved keyword translations
class_: Optional[str] = None
Expand Down Expand Up @@ -133,6 +134,7 @@ class Model(ml.Model):
"opt_model_no_refs1": Optional["ModelNoRefs1"],
"id": Optional[int],
"name": Optional[str],
"datetime_field": Optional[datetime.datetime],
"class_": Optional[str],
"finally_": Optional[Sequence[int]],
}
Expand All @@ -154,6 +156,7 @@ def __init__(
opt_model_no_refs1: Optional["ModelNoRefs1"] = None,
id: Optional[int] = None,
name: Optional[str] = None,
datetime_field: Optional[datetime.datetime] = None,
class_: Optional[str] = None,
finally_: Optional[Sequence[int]] = None,
):
Expand All @@ -175,13 +178,13 @@ def __init__(
self.opt_model_no_refs1 = opt_model_no_refs1
self.id = id
self.name = name
self.datetime_field = datetime_field
self.class_ = class_
self.finally_ = finally_


class Enum2(enum.Enum):
"""Post defined enum, used as ForwardRef.
"""
"""Post defined enum, used as ForwardRef."""

entry2 = "entry2"
invalid_api_enum_value = "invalid_api_enum_value"
Expand Down Expand Up @@ -218,8 +221,11 @@ def __init__(self, *, name2: str):
converter.register_structure_hook(ForwardRef("ModelNoRefs1"), structure_hook)
converter.register_structure_hook(ForwardRef("ModelNoRefs2"), structure_hook)
converter.register_structure_hook(Model, translate_keys_structure_hook)
converter.register_structure_hook(datetime.datetime, sr.datetime_structure_hook)


DATETIME_VALUE = datetime.datetime.fromtimestamp(1625246159, datetime.timezone.utc)
DATETIME_VALUE_STR = DATETIME_VALUE.strftime("%Y-%m-%dT%H:%M:%S.%f%z")
MODEL_DATA = {
"enum1": "entry1",
"model_no_refs1": {"name1": "model_no_refs1_name"},
Expand All @@ -231,6 +237,7 @@ def __init__(self, *, name2: str):
"opt_model_no_refs1": {"name1": "model_no_refs1_name"},
"id": 1,
"name": "my-name",
"datetime_field": DATETIME_VALUE_STR,
"class": "model-name",
"finally": [1, 2, 3],
}
Expand All @@ -249,6 +256,7 @@ def bm():
opt_model_no_refs1=None,
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand Down Expand Up @@ -357,6 +365,7 @@ def test_dict_iter(bm):
"opt_enum1",
"id",
"name",
"datetime_field",
"class",
"finally",
]
Expand All @@ -383,6 +392,7 @@ def test_dict_keys(bm):
"opt_enum1",
"id",
"name",
"datetime_field",
"class",
"finally",
]
Expand All @@ -399,6 +409,7 @@ def test_dict_items(bm):
("opt_enum1", "entry1"),
("id", 1),
("name", "my-name"),
("datetime_field", DATETIME_VALUE_STR),
("class", "model-name"),
("finally", [1, 2, 3]),
]
Expand All @@ -415,6 +426,7 @@ def test_dict_values(bm):
"entry1",
1,
"my-name",
DATETIME_VALUE_STR,
"model-name",
[1, 2, 3],
]
Expand Down Expand Up @@ -515,6 +527,7 @@ def test_deserialize_single() -> None:
opt_model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand All @@ -539,6 +552,7 @@ def test_deserialize_list():
opt_model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
),
Expand All @@ -563,6 +577,7 @@ def test_deserialize_partial():
opt_model_no_refs1=None,
id=None,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand All @@ -588,6 +603,7 @@ def test_deserialize_with_null():
opt_model_no_refs1=None,
id=None,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand Down Expand Up @@ -620,6 +636,7 @@ def test_serialize_single():
opt_model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand All @@ -639,6 +656,7 @@ def test_serialize_sequence():
opt_model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand All @@ -647,8 +665,7 @@ def test_serialize_sequence():


def test_serialize_partial():
"""Do not send json null for model None field values.
"""
"""Do not send json null for model None field values."""
model = Model(
enum1=Enum1.entry1,
model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
Expand All @@ -671,8 +688,7 @@ def test_serialize_partial():


def test_serialize_explict_null():
"""Send json null for model field EXPLICIT_NULL values.
"""
"""Send json null for model field EXPLICIT_NULL values."""
# pass EXPLICIT_NULL into constructor
model = Model(
enum1=Enum1.entry1,
Expand Down Expand Up @@ -719,6 +735,7 @@ def test_safe_enum_deserialization():
opt_model_no_refs1=ModelNoRefs1(name1="model_no_refs1_name"),
id=1,
name="my-name",
datetime_field=DATETIME_VALUE,
class_="model-name",
finally_=[1, 2, 3],
)
Expand Down