Skip to content

Commit e627f4a

Browse files
committed
fix: remove python version upper bound
fixes #944 the main fix here is switching from `cattr.register_structure_hook` to `cattr.register_structure_hook_func`. I re-organized some of the code in an effort to reproduce the issue in the test_serialize.py suite but I could not reproduce there, only in integration tests. Now that it's a function, we don't need a generated hook per type. I further isolated the converter usage switching the serializing to use each 31/40 dedicated converter. I also switched to the newer GenConverter. setup testing against python 3.10
1 parent 8bf7faf commit e627f4a

18 files changed

+210
-2429
lines changed

Diff for: .github/workflows/python-ci.yml

+6-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
- uses: actions/checkout@v2
3535
- uses: actions/setup-python@v2
3636
with:
37-
python-version: 3.9.9
37+
python-version: '3.10'
3838
- run: pip install -e .
3939
- run: pip install mypy types-requests
4040
- run: mypy looker_sdk/
@@ -59,14 +59,16 @@ jobs:
5959
- macos
6060
- windows
6161
python-version:
62-
- '3.9.9'
62+
- '3.10'
6363
include:
6464
- python-version: '3.6'
6565
os: ubuntu
6666
- python-version: '3.7'
6767
os: ubuntu
6868
- python-version: '3.8'
6969
os: ubuntu
70+
- python-version: '3.9'
71+
os: ubuntu
7072

7173
steps:
7274
- name: Repo Checkout
@@ -185,7 +187,7 @@ jobs:
185187
pip install tox
186188
187189
- name: Determine credentials version
188-
# Prior to 21_18, each version had different credentials and a
190+
# Prior to 21_18, each version had different credentials and a
189191
# different secret. 21_20 and later all use the same credentials
190192
# as 21_18. The parse_version.sh script parses the version and
191193
# yields 21_12, 21_14, 21_16 etc for those versions but 21_18 for
@@ -229,7 +231,7 @@ jobs:
229231
- uses: actions/checkout@v2
230232
- uses: actions/setup-python@v2
231233
with:
232-
python-version: 3.9.9
234+
python-version: '3.10'
233235
- name: Twine upload check
234236
run: |
235237
pip install wheel twine

Diff for: .github/workflows/python-publish.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929

3030
- uses: actions/setup-python@v2
3131
with:
32-
python-version: 3.9.9
32+
python-version: 3.10
3333

3434
- name: Package release artifacts
3535
run: |

Diff for: packages/sdk-codegen/src/python.gen.spec.ts

+6-10
Original file line numberDiff line numberDiff line change
@@ -88,19 +88,15 @@ class LookerSDK(api_methods.APIMethods):
8888
const type = apiTestModel.types.LookmlModelExploreJoins
8989
gen.declareType(indent, type)
9090
expect(gen.modelsEpilogue('')).toEqual(`
91-
92-
# The following cattrs structure hook registrations are a workaround
93-
# for https://github.com/Tinche/cattrs/pull/42 Once this issue is resolved
94-
# these calls will be removed.
95-
9691
import functools # noqa:E402
9792
98-
forward_ref_structure_hook = functools.partial(sr.forward_ref_structure_hook, globals(), sr.converter)
99-
translate_keys_structure_hook = functools.partial(sr.translate_keys_structure_hook, sr.converter)
100-
sr.converter.register_structure_hook(
101-
ForwardRef("LookmlModelExploreJoins"), # type: ignore
102-
forward_ref_structure_hook # type:ignore
93+
forward_ref_structure_hook = functools.partial(
94+
sr.forward_ref_structure_hook, globals(), sr.converter
95+
)
96+
sr.converter.register_structure_hook_func(
97+
lambda t: t.__class__ is ForwardRef, forward_ref_structure_hook
10398
)
99+
translate_keys_structure_hook = functools.partial(sr.translate_keys_structure_hook, sr.converter)
104100
sr.converter.register_structure_hook(
105101
LookmlModelExploreJoins, # type: ignore
106102
translate_keys_structure_hook # type:ignore

Diff for: packages/sdk-codegen/src/python.gen.ts

+6-15
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ export class PythonGen extends CodeGen {
100100

101101
// cattrs [un]structure hooks for model [de]serialization
102102
hooks: string[] = []
103-
structureHookFR = 'forward_ref_structure_hook'
104103
structureHookTK = 'translate_keys_structure_hook'
105104
pythonReservedKeywordClasses: Set<string> = new Set()
106105

@@ -141,18 +140,14 @@ DelimSequence = model.DelimSequence
141140
`
142141

143142
modelsEpilogue = (_indent: string) => `
144-
145-
# The following cattrs structure hook registrations are a workaround
146-
# for https://github.com/Tinche/cattrs/pull/42 Once this issue is resolved
147-
# these calls will be removed.
148-
149143
import functools # noqa:E402
150144
151-
${
152-
this.structureHookFR
153-
} = functools.partial(sr.forward_ref_structure_hook, globals(), sr.converter${
154-
this.apiRef
155-
})
145+
forward_ref_structure_hook = functools.partial(
146+
sr.forward_ref_structure_hook, globals(), sr.converter${this.apiRef}
147+
)
148+
sr.converter${this.apiRef}.register_structure_hook_func(
149+
lambda t: t.__class__ is ForwardRef, forward_ref_structure_hook
150+
)
156151
${
157152
this.structureHookTK
158153
} = functools.partial(sr.translate_keys_structure_hook, sr.converter${
@@ -476,10 +471,6 @@ ${this.hooks.join('\n')}
476471
}
477472
}
478473

479-
const forwardRef = `ForwardRef("${type.name}")`
480-
this.hooks.push(
481-
`sr.converter${this.apiRef}.register_structure_hook(\n${bump}${forwardRef}, # type: ignore\n${bump}${this.structureHookFR} # type:ignore\n)`
482-
)
483474
if (usesReservedPythonKeyword) {
484475
this.hooks.push(
485476
`sr.converter${this.apiRef}.register_structure_hook(\n${bump}${type.name}, # type: ignore\n${bump}${this.structureHookTK} # type:ignore\n)`

Diff for: python/.python-version

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
3.10.2
2+
3.9.10
13
3.9.9
24
3.9.0
35
3.8.2

Diff for: python/Pipfile

+1-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ cattrs = ">=1.3"
3131
python-dateutil = "*"
3232

3333
[requires]
34-
# pin python at 3.9.9 due to https://github.com/looker-open-source/sdk-codegen/issues/944
35-
python_full_version = "3.9.9"
34+
python_full_version = "3.10"
3635

3736
[pipenv]
3837
# for `black`

Diff for: python/looker_sdk/__init__.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def init31(
6262
return methods31.Looker31SDK(
6363
auth_session.AuthSession(settings, transport, serialize.deserialize31, "3.1"),
6464
serialize.deserialize31,
65-
serialize.serialize,
65+
serialize.serialize31,
6666
transport,
6767
"3.1",
6868
)
@@ -82,7 +82,7 @@ def init40(
8282
return methods40.Looker40SDK(
8383
auth_session.AuthSession(settings, transport, serialize.deserialize40, "4.0"),
8484
serialize.deserialize40,
85-
serialize.serialize,
85+
serialize.serialize40,
8686
transport,
8787
"4.0",
8888
)

Diff for: python/looker_sdk/rtl/api_methods.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def _get_serialized(self, body: TBody) -> Optional[bytes]:
147147
if isinstance(body, str):
148148
serialized = body.encode("utf-8")
149149
elif isinstance(body, (list, dict, model.Model)):
150-
serialized = self.serialize(body)
150+
serialized = self.serialize(api_model=body) # type: ignore
151151
else:
152152
serialized = None
153153
return serialized

Diff for: python/looker_sdk/rtl/auth_session.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ def _request_token(
317317
response = self.transport.request(
318318
transport.HttpMethod.POST,
319319
urllib.parse.urljoin(self.settings.base_url, "/api/token"),
320-
body=self.serialize(grant_type),
320+
body=self.serialize(api_model=grant_type), # type: ignore
321321
)
322322
if not response.ok:
323323
raise error.SDKError(response.value.decode(encoding=response.encoding))

Diff for: python/looker_sdk/rtl/hooks.py

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# The MIT License (MIT)
2+
#
3+
# Copyright (c) 2022 Looker Data Sciences, Inc.
4+
#
5+
# Permission is hereby granted, free of charge, to any person obtaining a copy
6+
# of this software and associated documentation files (the "Software"), to deal
7+
# in the Software without restriction, including without limitation the rights
8+
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
# copies of the Software, and to permit persons to whom the Software is
10+
# furnished to do so, subject to the following conditions:
11+
#
12+
# The above copyright notice and this permission notice shall be included in
13+
# all copies or substantial portions of the Software.
14+
#
15+
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
21+
# THE SOFTWARE.
22+
23+
import datetime
24+
import enum
25+
import keyword
26+
import sys
27+
from typing import Type
28+
29+
30+
def unstructure_hook(converter, api_model):
31+
"""cattr unstructure hook
32+
33+
Map reserved_ words in models to correct json field names.
34+
Also handle stripping None fields from dict while setting
35+
EXPLICIT_NULL fields to None so that we only send null
36+
in the json for fields the caller set EXPLICIT_NULL on.
37+
"""
38+
data = converter.unstructure_attrs_asdict(api_model)
39+
for key, value in data.copy().items():
40+
if value is None:
41+
del data[key]
42+
elif value == "EXPLICIT_NULL":
43+
data[key] = None
44+
# bug here: in the unittests cattrs unstructures this correctly
45+
# as an enum calling .value but in the integration tests we see
46+
# it doesn't for WriteCreateQueryTask.result_format for some reason
47+
# Haven't been able to debug it fully, so catching and processing
48+
# it here.
49+
elif isinstance(value, enum.Enum):
50+
data[key] = value.value
51+
for reserved in keyword.kwlist:
52+
if f"{reserved}_" in data:
53+
data[reserved] = data.pop(f"{reserved}_")
54+
return data
55+
56+
57+
DATETIME_FMT = "%Y-%m-%dT%H:%M:%S.%f%z"
58+
if sys.version_info < (3, 7):
59+
from dateutil import parser
60+
61+
def datetime_structure_hook(
62+
d: str, t: Type[datetime.datetime]
63+
) -> datetime.datetime:
64+
return parser.isoparse(d)
65+
66+
else:
67+
68+
def datetime_structure_hook(
69+
d: str, t: Type[datetime.datetime]
70+
) -> datetime.datetime:
71+
return datetime.datetime.strptime(d, DATETIME_FMT)
72+
73+
74+
def datetime_unstructure_hook(dt):
75+
return dt.strftime(DATETIME_FMT)
76+
77+
78+
def tr_data_keys(data):
79+
"""Map top level json keys to model property names.
80+
81+
Currently this translates reserved python keywords like "from" => "from_"
82+
"""
83+
for reserved in keyword.kwlist:
84+
if reserved in data and isinstance(data, dict):
85+
data[f"{reserved}_"] = data.pop(reserved)
86+
return data

Diff for: python/looker_sdk/rtl/model.py

+27-14
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,41 @@
2424
"""
2525

2626
import collections
27+
import datetime
2728
import enum
29+
import functools
2830
import keyword
29-
from typing import Any, cast, Iterable, Sequence, Optional, TypeVar
31+
from typing import Any, Iterable, Optional, Sequence, TypeVar, cast
32+
33+
import cattr
34+
35+
from looker_sdk.rtl import hooks
3036

3137
try:
3238
from typing import ForwardRef # type: ignore
3339
except ImportError:
3440
from typing import _ForwardRef as ForwardRef # type: ignore
3541

36-
import cattr
37-
3842

3943
EXPLICIT_NULL = cast(Any, "EXPLICIT_NULL") # type:ignore
4044

4145

4246
class Model:
43-
"""Base model for all generated models.
44-
"""
47+
"""Base model for all generated models."""
48+
49+
def _get_converter(self):
50+
if not hasattr(self, "_converter"):
51+
converter = cattr.Converter()
52+
converter.register_unstructure_hook(
53+
datetime.datetime, hooks.datetime_unstructure_hook
54+
)
55+
uh = functools.partial(hooks.unstructure_hook, converter)
56+
converter.register_unstructure_hook(Model, uh) # type: ignore
57+
self._converter = converter
58+
return self._converter
4559

4660
def _key_to_attr(self, key):
47-
"""Appends the trailing _ to python reserved words.
48-
"""
61+
"""Appends the trailing _ to python reserved words."""
4962
if key[-1] == "_":
5063
raise KeyError(key)
5164
if key in keyword.kwlist:
@@ -95,7 +108,7 @@ def err(val):
95108
raise ValueError(err(value))
96109
value = enum_member
97110
elif issubclass(actual_type, Model):
98-
value = cattr.structure(value, actual_type)
111+
value = self._get_converter().structure(value, actual_type)
99112

100113
return setattr(self, key, value)
101114

@@ -104,22 +117,22 @@ def __delitem__(self, key):
104117
setattr(self, self._key_to_attr(key), None)
105118

106119
def __iter__(self):
107-
return iter(cattr.unstructure(self))
120+
return iter(self._get_converter().unstructure(self))
108121

109122
def __len__(self):
110-
return len(cattr.unstructure(self))
123+
return len(self._get_converter().unstructure(self))
111124

112125
def __contains__(self, key):
113-
return key in cattr.unstructure(self)
126+
return key in self._get_converter().unstructure(self)
114127

115128
def keys(self):
116-
return cattr.unstructure(self).keys()
129+
return self._get_converter().unstructure(self).keys()
117130

118131
def items(self):
119-
return cattr.unstructure(self).items()
132+
return self._get_converter().unstructure(self).items()
120133

121134
def values(self):
122-
return cattr.unstructure(self).values()
135+
return self._get_converter().unstructure(self).values()
123136

124137
def get(self, key, default=None):
125138
try:

0 commit comments

Comments
 (0)