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

fix: increase timeout to prevent doubled resources (DEV-3114) #698

Merged
merged 5 commits into from
Dec 29, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/dsp_tools/commands/xmlupload/models/sipi.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,5 @@ def upload_bitstream(self, filepath: Path) -> dict[str, Any]:
"""
with open(filepath, "rb") as bitstream_file:
files = {"file": (filepath.name, bitstream_file)}
timeout = 5 * 60
res = self.con.post(route="/upload", files=files, timeout=timeout)
res = self.con.post(route="/upload", files=files)
return res
2 changes: 0 additions & 2 deletions src/dsp_tools/utils/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ def post(
route: str,
jsondata: str | None = None,
files: dict[str, tuple[str, Any]] | None = None,
timeout: int | None = None,
) -> dict[str, Any]:
"""
Make a HTTP POST request to the server to which this connection has been established.
Expand All @@ -54,7 +53,6 @@ def post(
route: route that will be called on the server
jsondata: Valid JSON as string
files: files to be uploaded, if any
timeout: timeout of the HTTP request, or None if the default should be used
"""

def delete(
Expand Down
23 changes: 8 additions & 15 deletions src/dsp_tools/utils/connection_live.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ class ConnectionLive:
dump: bool = False
dump_directory = Path("HTTP requests")
token: Optional[str] = None
# downtimes of server-side services -> API still processes request
# -> retry too early has side effects (e.g. duplicated resources)
timeout_put_post: int = 30 * 60
timeout_get_delete: int = 20

def __post_init__(self) -> None:
"""
Expand Down Expand Up @@ -218,7 +222,6 @@ def post(
route: str,
jsondata: Optional[str] = None,
files: dict[str, tuple[str, Any]] | None = None,
timeout: int | None = None,
) -> dict[str, Any]:
"""
Make a HTTP POST request to the server to which this connection has been established.
Expand All @@ -227,16 +230,10 @@ def post(
route: route that will be called on the server
jsondata: Valid JSON as string
files: files to be uploaded, if any
timeout: timeout of the HTTP request, or None if the default should be used

Returns:
response from server
"""
# timeout must be high enough,
# otherwise the client can get a timeout error while the API is still processing the request
# in that case, the client's retry will have undesired side effects (e.g. duplicated resources),
# and the response of the original API call will be lost
timeout = timeout or 60
if not route.startswith("/"):
route = f"/{route}"
url = self.server + route
Expand All @@ -246,7 +243,7 @@ def post(
if self.token:
headers["Authorization"] = f"Bearer {self.token}"

request = partial(requests.post, url=url, headers=headers, timeout=timeout)
request = partial(requests.post, url=url, headers=headers, timeout=self.timeout_put_post)
if jsondata:
# if data is not encoded as bytes, issues can occur with non-ASCII characters,
# where the content-length of the request will turn out to be different from the actual length
Expand Down Expand Up @@ -296,7 +293,7 @@ def get(
lambda: requests.get(
url=url,
headers=headers,
timeout=20,
timeout=self.timeout_get_delete,
)
)
if self.dump:
Expand Down Expand Up @@ -328,10 +325,6 @@ def put(
Returns:
response from server
"""
# timeout must be high enough,
# otherwise the client can get a timeout error while the API is still processing the request
# in that case, the client's retry will fail, and the response of the original API call will be lost
timeout = 60
if not route.startswith("/"):
route = f"/{route}"
url = self.server + route
Expand All @@ -348,7 +341,7 @@ def put(
# if data is not encoded as bytes, issues can occur with non-ASCII characters,
# where the content-length of the request will turn out to be different from the actual length
data=jsondata.encode("utf-8") if jsondata else None,
timeout=timeout,
timeout=self.timeout_put_post,
)
)
if self.dump:
Expand Down Expand Up @@ -388,7 +381,7 @@ def delete(
url=url,
headers=headers,
params=params,
timeout=20,
timeout=self.timeout_get_delete,
)
if self.dump:
self._write_request_to_file(
Expand Down
1 change: 0 additions & 1 deletion test/unittests/commands/xmlupload/connection_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ def post(
route: str,
jsondata: str | None = None,
files: dict[str, tuple[str, Any]] | None = None,
timeout: int | None = None,
) -> dict[str, Any]:
raise AssertionError("POST not implemented in mock")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ def post(
route: str,
jsondata: str | None = None,
files: dict[str, tuple[str, Any]] | None = None,
timeout: int | None = None,
) -> dict[str, Any]:
return self.post_responses.pop(0)

Expand Down