Skip to content

Commit 3240637

Browse files
authored
Python: Fix regressions in the Python library by cleaning up the templates (#970)
I haven't tracked down the first version where this failed, but I believe this started when we updated the openapi python client generator. I think they heavily changed the templates which made this fail miserably. This whole thing originated from a bug report from a customer about endpoint.list() not working because EndponitOut was not imported (which is because of the changes to the templates). The solution was to bring in the templates from the generator at version 0.14.1 (the one we use), and bring in all the changes that we wanted to have. I also noticed while looking at it, that the retry mechanism was buggy. We probably wouldn't have hit it in production scenarios, as it required calls to the same model to error repeatedly, but it was reusing the global variable and increasing the timeout all the time.
2 parents 8982cbb + 3b1d3a1 commit 3240637

File tree

9 files changed

+175
-103
lines changed

9 files changed

+175
-103
lines changed

.github/workflows/python-lint.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
- uses: actions/setup-python@v2
1414
name: Install Python
1515
with:
16-
python-version: '3.9'
16+
python-version: '3.11'
1717

1818
- name: Install deps
1919
run: |
@@ -30,4 +30,4 @@ jobs:
3030
- name: Run linting
3131
run: |
3232
cd python
33-
sh ./scripts/lint.sh
33+
sh ./scripts/lint.sh

.github/workflows/python-release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
- uses: actions/setup-python@v2
1616
name: Install Python
1717
with:
18-
python-version: '3.8'
18+
python-version: '3.11'
1919

2020
- name: Install deps
2121
run: |
@@ -43,4 +43,4 @@ jobs:
4343
with:
4444
user: __token__
4545
password: ${{ secrets.TWINE_PASSWORD }}
46-
packages_dir: python/dist
46+
packages_dir: python/dist

python/requirements-dev.txt

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# This file is autogenerated by pip-compile with Python 3.10
2+
# This file is autogenerated by pip-compile with Python 3.11
33
# by the following command:
44
#
55
# pip-compile --output-file=requirements-dev.txt requirements.in/development.txt
@@ -62,13 +62,13 @@ markupsafe==2.1.0
6262
# via jinja2
6363
mccabe==0.6.1
6464
# via flake8
65-
mypy==0.931
65+
mypy==1.4.0
6666
# via -r requirements.in/development.txt
67-
mypy-extensions==0.4.3
67+
mypy-extensions==1.0.0
6868
# via
6969
# black
7070
# mypy
71-
openapi-python-client==0.14.0
71+
openapi-python-client==0.14.1
7272
# via -r requirements.in/development.txt
7373
packaging==23.1
7474
# via
@@ -91,7 +91,7 @@ pycodestyle==2.7.0
9191
# via
9292
# flake8
9393
# flake8-print
94-
pydantic==1.9.0
94+
pydantic==1.10.0
9595
# via openapi-python-client
9696
pyflakes==2.3.1
9797
# via
@@ -120,15 +120,9 @@ sniffio==1.2.0
120120
# httpx
121121
toml==0.10.2
122122
# via pytest
123-
tomli==2.0.0
124-
# via
125-
# black
126-
# build
127-
# mypy
128-
# pyproject-hooks
129123
typer==0.7.0
130124
# via openapi-python-client
131-
typing-extensions==3.10.0.0
125+
typing-extensions==4.6.3
132126
# via
133127
# mypy
134128
# pydantic

python/requirements.in/development.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ isort
44
flake8
55
flake8-print
66
pep8-naming
7-
mypy
7+
mypy>=1.4.0
88
pip-tools>=6.13.0
99
pytest
1010
httpx>=0.23.0
11-
openapi-python-client>=0.14.0
11+
openapi-python-client>=0.14.1

python/svix/api.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,8 @@ def __init__(self, auth_token: str, options: SvixOptions = SvixOptions()) -> Non
993993
headers={"user-agent": f"svix-libs/{__version__}/python"},
994994
verify_ssl=True,
995995
timeout=15,
996+
follow_redirects=False,
997+
raise_on_unexpected_status=True,
996998
)
997999
self._client = client
9981000

python/templates/endpoint_macros.py.jinja

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{% from "property_templates/helpers.jinja" import guarded_statement %}
2+
{% from "helpers.jinja" import safe_docstring %}
23

34
{% macro header_params(endpoint) %}
45
{% if endpoint.header_parameters %}
@@ -92,8 +93,8 @@ client: AuthenticatedClient,
9293
client: Client,
9394
{% endif %}
9495
{# Form data if any #}
95-
{% if endpoint.form_body_class %}
96-
form_data: {{ endpoint.form_body_class.name }},
96+
{% if endpoint.form_body %}
97+
form_data: {{ endpoint.form_body.get_type_string() }},
9798
{% endif %}
9899
{# Multipart data if any #}
99100
{% if endpoint.multipart_body %}
@@ -122,7 +123,7 @@ json_body: {{ endpoint.json_body.get_type_string() }},
122123
{{ parameter.python_name }}={{ parameter.python_name }},
123124
{% endfor %}
124125
client=client,
125-
{% if endpoint.form_body_class %}
126+
{% if endpoint.form_body %}
126127
form_data=form_data,
127128
{% endif %}
128129
{% if endpoint.multipart_body %}
@@ -142,8 +143,8 @@ json_body=json_body,
142143
{% endfor %}
143144
{% endmacro %}
144145

145-
{% macro docstring(endpoint, return_string) %}
146-
"""{% if endpoint.summary %}{{ endpoint.summary | wordwrap(100)}}
146+
{% macro docstring_content(endpoint, return_string, is_detailed) %}
147+
{% if endpoint.summary %}{{ endpoint.summary | wordwrap(100)}}
147148

148149
{% endif -%}
149150
{%- if endpoint.description %} {{ endpoint.description | wordwrap(100) }}
@@ -161,7 +162,18 @@ Args:
161162
{% endfor %}
162163

163164
{% endif %}
165+
Raises:
166+
errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True.
167+
httpx.TimeoutException: If the request takes longer than Client.timeout.
168+
164169
Returns:
170+
{% if is_detailed %}
171+
Response[{{ return_string }}]
172+
{% else %}
165173
{{ return_string }}
166-
"""
167-
{% endmacro %}
174+
{% endif %}
175+
{% endmacro %}
176+
177+
{% macro docstring(endpoint, return_string, is_detailed) %}
178+
{{ safe_docstring(docstring_content(endpoint, return_string, is_detailed)) }}
179+
{% endmacro %}
Lines changed: 54 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,26 @@
11
from http import HTTPStatus
2-
from typing import Any, Dict, List, Optional, Union, cast
2+
from typing import Any, Dict, List, Union, cast
33
from time import sleep
44
import random
55

66
import httpx
77

88
from ...client import AuthenticatedClient, Client
99
from ...types import Response, UNSET
10+
from ... import errors
1011

1112
{% for relative in endpoint.relative_imports %}
1213
{{ relative }}
1314
{% endfor %}
1415

16+
SLEEP_TIME = 0.05
17+
NUM_RETRIES = 3
18+
1519
{% from "endpoint_macros.py.jinja" import header_params, cookie_params, query_params, json_body, multipart_body,
1620
arguments, client, kwargs, parse_response, docstring %}
1721

18-
{% set return_string = endpoint.response_type() %}
19-
{% set return_type = endpoint.responses[0].prop.get_type_string() if endpoint.responses[0].prop.get_type_string() != "Any" else None%}
20-
{% set parsed_responses = (endpoint.responses | length > 0) and return_string != "Any" %}
22+
{% set return_string = endpoint.responses[0].prop.get_type_string() if endpoint.responses[0].prop.get_type_string() != "Any" else "None" %}
23+
{% set parsed_responses = (endpoint.responses | length > 0) %}
2124

2225
def _get_kwargs(
2326
{{ arguments(endpoint) | indent(4) }}
@@ -48,7 +51,8 @@ def _get_kwargs(
4851
"headers": headers,
4952
"cookies": cookies,
5053
"timeout": client.get_timeout(),
51-
{% if endpoint.form_body_class %}
54+
"follow_redirects": client.follow_redirects,
55+
{% if endpoint.form_body %}
5256
"data": form_data.to_dict(),
5357
{% elif endpoint.multipart_body %}
5458
"files": {{ "multipart_" + endpoint.multipart_body.python_name }},
@@ -61,70 +65,72 @@ def _get_kwargs(
6165
}
6266

6367

64-
{% if parsed_responses %}
65-
def _parse_response(*, response: httpx.Response) -> {{ return_type }}:
66-
{% for response in endpoint.responses if response.status_code >= 299 %}
67-
if response.status_code == {{ response.status_code }}:
68-
raise {{ response.prop.class_info.name }}({{ response.source }}, response.status_code)
68+
def _parse_response(*, client: Client, response: httpx.Response) -> {% if return_string == "None" %}Any{% else %}{{ return_string }}{% endif %}:
69+
{% for response in endpoint.responses %}
70+
if response.status_code == HTTPStatus.{{ response.status_code.name }}:
71+
{% import "property_templates/" + response.prop.template as prop_template %}
72+
{% if 200 <= response.status_code <= 299 %}
73+
{% if prop_template.construct %}
74+
{{ prop_template.construct(response.prop, response.source) | indent(8) }}
75+
{% else %}
76+
{{ response.prop.python_name }} = cast({{ response.prop.get_type_string() }}, {{ response.source }})
77+
{% endif %}
78+
return {{ response.prop.python_name }}
79+
{% else %}
80+
raise {{ response.prop.get_type_string() }}.init_exception({{ response.source }}, response.status_code)
81+
{% endif %}
6982
{% endfor %}
70-
{%set ok_response = endpoint.responses[0]%}
71-
{% import "property_templates/" + ok_response.prop.template as ok_response_prop_template %}
72-
{% if ok_response.prop and ok_response.prop.required_properties == [] and ok_response.prop.optional_properties == [] %}
73-
{{ ok_response.prop.python_name }} = {{ ok_response.prop.class_info.name }}({{ ok_response.source }})
74-
{% elif ok_response_prop_template.construct %}
75-
{{ ok_response_prop_template.construct(ok_response.prop, ok_response.source) | indent(8) }}
76-
{% else %}
77-
{{ ok_response.prop.python_name }} = None
78-
{% endif %}
79-
return {{ok_response.prop.python_name}}
80-
{% endif %}
83+
raise errors.UnexpectedStatus(response.status_code, response.content)
84+
85+
86+
def _build_response(*, client: Client, response: httpx.Response) -> Response[{{ return_string }}]:
87+
return Response(
88+
status_code=HTTPStatus(response.status_code),
89+
content=response.content,
90+
headers=response.headers,
91+
parsed=_parse_response(client=client, response=response),
92+
)
8193

82-
sleep_time = 0.05
83-
num_retries = 3
8494

8595
def sync_detailed(
8696
{{ arguments(endpoint) | indent(4) }}
87-
) -> {{return_type}}:
88-
{{ docstring(endpoint, return_type) | indent(4) }}
97+
) -> Response[{{ return_string }}]:
98+
{{ docstring(endpoint, return_string, is_detailed=true) | indent(4) }}
8999

90100
kwargs = _get_kwargs(
91101
{{ kwargs(endpoint) }}
92102
)
93103

94104
kwargs['headers'] = {'svix-req-id':f"{random.getrandbits(32)}", **kwargs['headers']}
95105

96-
retry_count = 0
97-
for retry in range(num_retries):
106+
for retry_count in range(NUM_RETRIES):
98107
response = httpx.request(
99108
verify=client.verify_ssl,
100109
**kwargs,
101110
)
102-
if response.status_code >= 500 and retry < num_retries:
103-
retry_count +=1
104-
kwargs['headers']['svix-retry-count']= str(retry_count)
105-
sleep(sleep_time)
106-
sleep_time = sleep_time * 2
111+
if response.status_code >= 500:
112+
kwargs['headers']['svix-retry-count'] = str(retry_count)
113+
sleep(SLEEP_TIME * (2 ** retry_count))
107114
else:
108115
break
109116

110-
111-
return _parse_response(response=response)
117+
return _build_response(client=client, response=response)
112118

113119
{% if parsed_responses %}
114120
def sync(
115121
{{ arguments(endpoint) | indent(4) }}
116-
) -> {{ return_type }}:
117-
{{ docstring(endpoint, return_type) | indent(4) }}
122+
) -> {{ return_string }}:
123+
{{ docstring(endpoint, return_string, is_detailed=false) | indent(4) }}
118124

119125
return sync_detailed(
120126
{{ kwargs(endpoint) }}
121-
)
127+
).parsed
122128
{% endif %}
123129

124130
async def asyncio_detailed(
125131
{{ arguments(endpoint) | indent(4) }}
126-
) -> {{ return_type }}:
127-
{{ docstring(endpoint, return_type) | indent(4) }}
132+
) -> Response[{{ return_string }}]:
133+
{{ docstring(endpoint, return_string, is_detailed=true) | indent(4) }}
128134

129135
kwargs = _get_kwargs(
130136
{{ kwargs(endpoint) }}
@@ -133,28 +139,25 @@ async def asyncio_detailed(
133139
kwargs['headers'] = {'svix-req-id':f"{random.getrandbits(32)}", **kwargs['headers']}
134140

135141
async with httpx.AsyncClient(verify=client.verify_ssl) as _client:
136-
retry_count = 0
137-
for retry in range(num_retries):
142+
for retry_count in range(NUM_RETRIES):
138143
response = await _client.request(
139144
**kwargs
140145
)
141-
if response.status_code >= 500 and retry < num_retries:
142-
retry_count +=1
143-
kwargs['headers']['svix-retry-count']= str(retry_count)
144-
sleep(sleep_time)
145-
sleep_time = sleep_time * 2
146+
if response.status_code >= 500:
147+
kwargs['headers']['svix-retry-count'] = str(retry_count)
148+
sleep(SLEEP_TIME * (2 ** retry_count))
146149
else:
147150
break
148151

149-
return _parse_response(response=response)
152+
return _build_response(client=client, response=response)
150153

151154
{% if parsed_responses %}
152155
async def asyncio(
153156
{{ arguments(endpoint) | indent(4) }}
154-
) -> {{ return_type }}:
155-
{{ docstring(endpoint, return_type) | indent(4) }}
157+
) -> {{ return_string }}:
158+
{{ docstring(endpoint, return_string, is_detailed=false) | indent(4) }}
156159

157160
return (await asyncio_detailed(
158161
{{ kwargs(endpoint) }}
159-
))
160-
{% endif %}
162+
)).parsed
163+
{% endif %}

0 commit comments

Comments
 (0)