From 50a60fde4e93bd3a1282652df7eeabcbbbce6886 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Mon, 27 Jul 2026 11:50:23 +0200 Subject: [PATCH 1/5] pw: let the lib handle POST and PATCH retries By default, these methods are not retried, because they are not considered to be idempotent: multiple requests with the same parameters end with the same state. Here in this patchwork module, it looks like it is fine to retry multiple times in case of errors. It sounds better doing that with the lib than manual code here, with different timeout. Note that the default manual retry timeout was too low: recently, the nipa-upload-wireless service failed because it wasn't able to post a check, even with 3 retries. All attempts failed with a 504 error. With the previous code, it was retrying only during 3.5 minutes, compared to ~17 minutes with the lib. That leaves more time for server maintenance operations. Signed-off-by: Matthieu Baerts --- pw/patchwork.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pw/patchwork.py b/pw/patchwork.py index e75a547f..4b7795cc 100644 --- a/pw/patchwork.py +++ b/pw/patchwork.py @@ -32,8 +32,9 @@ class PatchworkPostException(Exception): class Patchwork(object): def __init__(self, config): self._session = requests.Session() + allowed_methods = Retry.DEFAULT_ALLOWED_METHODS | {'POST', 'PATCH'} retry = Retry(connect=10, status=10, status_forcelist={502, 504}, - backoff_factor=1) + allowed_methods=allowed_methods, backoff_factor=1) adapter = HTTPAdapter(max_retries=retry) self._session.mount('http://', adapter) self._session.mount('https://', adapter) @@ -232,13 +233,6 @@ def post_check(self, patch, name, state, url, desc): } r = self._post(f'patches/{patch}/checks/', headers=headers, data=data) - for retry in range(3): - if r.status_code == 502 or r.status_code == 504: - # Timeout, let's wait 30 sec and retry, POST isn't retried by the lib. - time.sleep(30 << retry) - r = self._post(f'patches/{patch}/checks/', headers=headers, data=data) - else: - break if r.status_code != 201: raise PatchworkPostException(r) From 121a616ce2bde99b3abf6b74434312144fae1d4a Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Mon, 27 Jul 2026 11:53:47 +0200 Subject: [PATCH 2/5] pw: patch: fix typo in log s/post/patch/, to be able to distinct between post and patch. Signed-off-by: Matthieu Baerts --- pw/patchwork.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pw/patchwork.py b/pw/patchwork.py index 4b7795cc..d006f833 100644 --- a/pw/patchwork.py +++ b/pw/patchwork.py @@ -168,7 +168,7 @@ def _post(self, req, headers, data, api='1.1'): def _patch(self, req, headers, data, api='1.1'): url = f'{self._proto}{self.server}/api/{api}/{req}' try: - core.log_open_sec(f"Patchwork {self.server} post: {url}") + core.log_open_sec(f"Patchwork {self.server} patch: {url}") ret = self._session.patch(url, headers=headers, data=data) core.log("Headers", headers) core.log("Data", data) From 12f382d89f73a1e20546274be49364c130fb4921 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Mon, 27 Jul 2026 11:54:35 +0200 Subject: [PATCH 3/5] pw: patch: handle non JSON reponse data This can probably happen in case of errors, e.g. a 504 timeout error. Signed-off-by: Matthieu Baerts --- pw/patchwork.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pw/patchwork.py b/pw/patchwork.py index d006f833..0ef0c2b1 100644 --- a/pw/patchwork.py +++ b/pw/patchwork.py @@ -173,7 +173,10 @@ def _patch(self, req, headers, data, api='1.1'): core.log("Headers", headers) core.log("Data", data) core.log("Response", ret) - core.log("Response data", ret.json()) + try: + core.log("Response data", ret.json()) + except json.decoder.JSONDecodeError: + core.log("Response data", ret.content.decode()) finally: core.log_end_sec() From bb4c9c4baf71e7b3131f663e7ecd1b2c4b0c2f09 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Mon, 27 Jul 2026 12:18:38 +0200 Subject: [PATCH 4/5] pw: log POST/PATCH response time Now that retries are handled by the lib, it looks interesting to find out how long the different requests took, similar to what is done with the GET methods. Use a different message for each method, to be able to monitor them individually if needed. Note that on the brancher service, the longest GET response time was at 86 seconds. So logging such info looks important to understand other errors. Signed-off-by: Matthieu Baerts --- pw/patchwork.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pw/patchwork.py b/pw/patchwork.py index 0ef0c2b1..b46a8d1e 100644 --- a/pw/patchwork.py +++ b/pw/patchwork.py @@ -72,7 +72,7 @@ def _request(self, url): core.log("Response data", ret.content.decode()) finally: end = datetime.datetime.now() - core.log("Response time (sec)", (end - start).total_seconds()) + core.log("Response time GET (sec)", (end - start).total_seconds()) core.log_end_sec() return ret @@ -149,8 +149,10 @@ def _get(self, req, api='1.1'): def _post(self, req, headers, data, api='1.1'): url = f'{self._proto}{self.server}/api/{api}/{req}' + core.log_open_sec(f"Patchwork {self.server} post: {url}") + start = datetime.datetime.now() + try: - core.log_open_sec(f"Patchwork {self.server} post: {url}") ret = self._session.post(url, headers=headers, data=data) core.log("Headers", headers) core.log("Data", data) @@ -160,6 +162,8 @@ def _post(self, req, headers, data, api='1.1'): except json.decoder.JSONDecodeError: core.log("Response data", ret.content.decode()) finally: + end = datetime.datetime.now() + core.log("Response time POST (sec)", (end - start).total_seconds()) core.log_end_sec() return ret @@ -167,8 +171,10 @@ def _post(self, req, headers, data, api='1.1'): # PATCH as in the HTTP method, not getting a patch def _patch(self, req, headers, data, api='1.1'): url = f'{self._proto}{self.server}/api/{api}/{req}' + core.log_open_sec(f"Patchwork {self.server} patch: {url}") + start = datetime.datetime.now() + try: - core.log_open_sec(f"Patchwork {self.server} patch: {url}") ret = self._session.patch(url, headers=headers, data=data) core.log("Headers", headers) core.log("Data", data) @@ -178,6 +184,8 @@ def _patch(self, req, headers, data, api='1.1'): except json.decoder.JSONDecodeError: core.log("Response data", ret.content.decode()) finally: + end = datetime.datetime.now() + core.log("Response time PATCH (sec)", (end - start).total_seconds()) core.log_end_sec() return ret From 9e7c5b1a4bbd1d9e7dff2a04ba7a78322e0c0509 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Mon, 27 Jul 2026 12:24:53 +0200 Subject: [PATCH 5/5] pw: log GET/POST/PATCH start timestamps In case of issues, it looks interesting to have the timestamps in the logs, to find out a root cause, e.g. a server maintenance, or too many parallel requests. Signed-off-by: Matthieu Baerts --- pw/patchwork.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pw/patchwork.py b/pw/patchwork.py index b46a8d1e..37cddf0e 100644 --- a/pw/patchwork.py +++ b/pw/patchwork.py @@ -62,6 +62,7 @@ def __init__(self, config): def _request(self, url): core.log_open_sec(f"Patchwork {self.server} request: {url}") start = datetime.datetime.now() + core.log("Start", start) try: ret = self._session.get(url) @@ -151,6 +152,7 @@ def _post(self, req, headers, data, api='1.1'): url = f'{self._proto}{self.server}/api/{api}/{req}' core.log_open_sec(f"Patchwork {self.server} post: {url}") start = datetime.datetime.now() + core.log("Start", start) try: ret = self._session.post(url, headers=headers, data=data) @@ -173,6 +175,7 @@ def _patch(self, req, headers, data, api='1.1'): url = f'{self._proto}{self.server}/api/{api}/{req}' core.log_open_sec(f"Patchwork {self.server} patch: {url}") start = datetime.datetime.now() + core.log("Start", start) try: ret = self._session.patch(url, headers=headers, data=data)