From f23c9524ba65a752827f3ba0348234548c2b625d Mon Sep 17 00:00:00 2001 From: Maja Massarini Date: Tue, 19 Dec 2023 14:58:00 +0100 Subject: [PATCH] Fix race condition causing ENOPRSTATS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nikola Forró --- ogr/abstract.py | 7 +- ogr/services/pagure/project.py | 7 +- ogr/services/pagure/pull_request.py | 56 ++++-- ...=> PullRequests.test_pr_diff_failing.yaml} | 0 ...ts.test_pr_diff_failing_and_succeding.yaml | 169 ++++++++++++++++++ .../integration/pagure/test_pull_requests.py | 12 +- 6 files changed, 234 insertions(+), 17 deletions(-) rename tests/integration/pagure/test_data/test_pull_requests/{PullRequests.test_pr_diff_empty_diff.yaml => PullRequests.test_pr_diff_failing.yaml} (100%) create mode 100644 tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing_and_succeding.yaml diff --git a/ogr/abstract.py b/ogr/abstract.py index 117d39a5..0d56b87e 100644 --- a/ogr/abstract.py +++ b/ogr/abstract.py @@ -1688,7 +1688,12 @@ def get_pr(self, pr_id: int) -> "PullRequest": """ raise NotImplementedError() - def get_pr_files_diff(self, pr_id: int) -> dict: + def get_pr_files_diff( + self, + pr_id: int, + retries: int = 0, + wait_seconds: int = 3, + ) -> dict: """ Get files diff of a pull request. diff --git a/ogr/services/pagure/project.py b/ogr/services/pagure/project.py index c0938b95..1deb6060 100644 --- a/ogr/services/pagure/project.py +++ b/ogr/services/pagure/project.py @@ -276,7 +276,12 @@ def get_pr(self, pr_id: int) -> PullRequest: pass @indirect(PagurePullRequest.get_files_diff) - def get_pr_files_diff(self, pr_id: int) -> dict: + def get_pr_files_diff( + self, + pr_id: int, + retries: int = 0, + wait_seconds: int = 3, + ) -> dict: pass @if_readonly(return_function=GitProjectReadOnly.create_pr) diff --git a/ogr/services/pagure/pull_request.py b/ogr/services/pagure/pull_request.py index 6f86a61c..38ea0ef4 100644 --- a/ogr/services/pagure/pull_request.py +++ b/ogr/services/pagure/pull_request.py @@ -3,6 +3,7 @@ import datetime import logging +from time import sleep from typing import Any, Optional, Union from ogr.abstract import CommitFlag, CommitStatus, PRComment, PRStatus, PullRequest @@ -191,18 +192,49 @@ def get(project: "ogr_pagure.PagureProject", pr_id: int) -> "PullRequest": return PagurePullRequest(raw_pr, project) @staticmethod - def get_files_diff(project: "ogr_pagure.PagureProject", pr_id: int) -> dict: - try: - return project._call_project_api( - "pull-request", - str(pr_id), - "diffstats", - method="GET", - ) - except PagureAPIException as ex: - if "No statistics" in ex.pagure_error: - return {} - raise ex + def get_files_diff( + project: "ogr_pagure.PagureProject", + pr_id: int, + retries: int = 0, + wait_seconds: int = 3, + ) -> dict: + """ + Retrieve pull request diff statistics. + + Pagure API tends to return ENOPRSTATS error when a pull request is transitioning + from open to other states, so you can use `retries` and `wait_seconds` to try to + mitigate that. + + + Args: + project: Pagure project. + pr_id: Pull request ID. + retries: Number of extra attempts. + wait_seconds: Delay between attempts. + """ + attempt = 0 + while True: + try: + return project._call_project_api( + "pull-request", + str(pr_id), + "diffstats", + method="GET", + ) + except PagureAPIException as ex: # noqa PERF203 + if "No statistics" in ex.pagure_error: + # this may be a race condition, try once more + logger.error( + f"While retrieving PR diffstats Pagure returned ENOPRSTATS. \n{ex}", + ) + if attempt < retries: + logger.debug( + f"Trying again; attempt={attempt} after {wait_seconds}seconds", + ) + attempt += 1 + sleep(wait_seconds) + continue + raise ex @staticmethod def get_list( diff --git a/tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_empty_diff.yaml b/tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing.yaml similarity index 100% rename from tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_empty_diff.yaml rename to tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing.yaml diff --git a/tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing_and_succeding.yaml b/tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing_and_succeding.yaml new file mode 100644 index 00000000..7e8b08d9 --- /dev/null +++ b/tests/integration/pagure/test_data/test_pull_requests/PullRequests.test_pr_diff_failing_and_succeding.yaml @@ -0,0 +1,169 @@ +_requre: + DataTypes: 1 + key_strategy: StorageKeysInspectSimple + version_storage_file: 3 +requests.sessions: + send: + GET: + https://pagure.io/api/0/ogr-tests/pull-request/7/diffstats: + - metadata: + latency: 0.2965199947357178 + module_call_list: + - unittest.case + - requre.record_and_replace + - tests.integration.pagure.test_pull_requests + - ogr.abstract + - ogr.utils + - ogr.abstract + - ogr.services.pagure.pull_request + - ogr.abstract + - ogr.services.pagure.project + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - requests.sessions + - requre.objects + - requre.cassette + - requests.sessions + - send + output: + __store_indicator: 2 + _content: + error: No statistics could be computed for this PR + error_code: ENOPRSTATS + _next: null + elapsed: 0.2 + encoding: utf-8 + headers: + Connection: close + Content-Length: '92' + Content-Security-Policy: default-src 'self';script-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne' + https://apps.fedoraproject.org; style-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne'; + object-src 'none';base-uri 'self';img-src 'self' https:; + Content-Type: application/json + Date: Fri, 01 Nov 2019 13-36-03 GMT + Referrer-Policy: same-origin + Server: Apache/2.4.37 (Red Hat Enterprise Linux) OpenSSL/1.1.1k mod_wsgi/4.6.4 + Python/3.6 + Set-Cookie: pagure=eyJfcGVybWFuZW50Ijp0cnVlLCJjc3JmX3Rva2VuIjoiMzU0YmExMTNlMWFhMGU0MzliZDFjN2UwZGY3ODg4OTBjODdhMzkwMSJ9.GFnGGw.MvcSSJ61ycB9FCodU3kj9hWZeGY; + Expires=Fri, 12-Jan-2024 10:23:23 GMT; Secure; HttpOnly; Path=/ + Strict-Transport-Security: max-age=31536000; includeSubDomains; preload + X-Content-Type-Options: nosniff + X-Frame-Options: ALLOW-FROM https://pagure.io/ + X-Xss-Protection: 1; mode=block + raw: !!binary "" + reason: BAD REQUEST + status_code: 400 + - metadata: + latency: 0.2961406707763672 + module_call_list: + - unittest.case + - requre.record_and_replace + - tests.integration.pagure.test_pull_requests + - ogr.abstract + - ogr.utils + - ogr.abstract + - ogr.services.pagure.pull_request + - ogr.abstract + - ogr.services.pagure.project + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - requests.sessions + - requre.objects + - requre.cassette + - requests.sessions + - send + output: + __store_indicator: 2 + _content: + README.md: + lines_added: 5 + lines_removed: 1 + new_id: 3953698d00a5c8c8ef0582a05083a242beca4033 + old_id: a40a52a2ba46ddf4f9a4ecd04b47f717573467a6 + old_path: README.md + status: M + _next: null + elapsed: 0.2 + encoding: utf-8 + headers: + Connection: Keep-Alive + Content-Length: '239' + Content-Security-Policy: default-src 'self';script-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne' + https://apps.fedoraproject.org; style-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne'; + object-src 'none';base-uri 'self';img-src 'self' https:; + Content-Type: application/json + Date: Fri, 01 Nov 2019 13-36-03 GMT + Keep-Alive: timeout=5, max=99 + Referrer-Policy: same-origin + Server: Apache/2.4.37 (Red Hat Enterprise Linux) OpenSSL/1.1.1k mod_wsgi/4.6.4 + Python/3.6 + Set-Cookie: pagure=eyJfcGVybWFuZW50Ijp0cnVlLCJjc3JmX3Rva2VuIjoiOGRjYjBkOTVmZWVhYTdjMjhjNmNkNWNhN2ZlZmFmNGE3ZjMxZDliMSJ9.GFnCeg.6mYem_Ppb-t6pkIsrmcLGfZk3jg; + Expires=Fri, 12-Jan-2024 10:07:54 GMT; Secure; HttpOnly; Path=/ + Strict-Transport-Security: max-age=31536000; includeSubDomains; preload + X-Content-Type-Options: nosniff + X-Frame-Options: ALLOW-FROM https://pagure.io/ + X-Xss-Protection: 1; mode=block + raw: !!binary "" + reason: OK + status_code: 200 + POST: + https://pagure.io/api/0/-/whoami: + - metadata: + latency: 0.565004825592041 + module_call_list: + - unittest.case + - requre.record_and_replace + - tests.integration.pagure.test_pull_requests + - tests.integration.pagure.base + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.user + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - ogr.abstract + - ogr.services.pagure.service + - requests.sessions + - requre.objects + - requre.cassette + - requests.sessions + - send + output: + __store_indicator: 2 + _content: + username: lbarczio + _next: null + elapsed: 0.2 + encoding: utf-8 + headers: + Connection: Upgrade, Keep-Alive + Content-Length: '29' + Content-Security-Policy: default-src 'self';script-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne' + https://apps.fedoraproject.org; style-src 'self' 'nonce-YqLDC0BS8d7iY8mKO7VtBbIne'; + object-src 'none';base-uri 'self';img-src 'self' https:; + Content-Type: application/json + Date: Fri, 01 Nov 2019 13-36-03 GMT + Keep-Alive: timeout=5, max=100 + Referrer-Policy: same-origin + Server: Apache/2.4.37 (Red Hat Enterprise Linux) OpenSSL/1.1.1k mod_wsgi/4.6.4 + Python/3.6 + Set-Cookie: pagure=eyJfcGVybWFuZW50Ijp0cnVlLCJjc3JmX3Rva2VuIjoiMzU0YmExMTNlMWFhMGU0MzliZDFjN2UwZGY3ODg4OTBjODdhMzkwMSJ9.GFnGGw.MvcSSJ61ycB9FCodU3kj9hWZeGY; + Expires=Fri, 12-Jan-2024 10:23:23 GMT; Secure; HttpOnly; Path=/ + Strict-Transport-Security: max-age=31536000; includeSubDomains; preload + Upgrade: h2,h2c + X-Content-Type-Options: nosniff + X-Frame-Options: ALLOW-FROM https://pagure.io/ + X-Xss-Protection: 1; mode=block + raw: !!binary "" + reason: OK + status_code: 200 diff --git a/tests/integration/pagure/test_pull_requests.py b/tests/integration/pagure/test_pull_requests.py index 2015657c..9d2b6e42 100644 --- a/tests/integration/pagure/test_pull_requests.py +++ b/tests/integration/pagure/test_pull_requests.py @@ -1,9 +1,11 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT +import pytest from requre.online_replacing import record_requests_for_all_methods from ogr.abstract import CommitStatus, PRStatus +from ogr.exceptions import PagureAPIException from tests.integration.pagure.base import PagureTests @@ -124,7 +126,11 @@ def test_pr_diff(self): assert isinstance(diff, dict) assert "README.md" in diff - def test_pr_diff_empty_diff(self): - diff = self.ogr_project.get_pr_files_diff(6) + def test_pr_diff_failing(self): + with pytest.raises(PagureAPIException): + self.ogr_project.get_pr_files_diff(6) + + def test_pr_diff_failing_and_succeding(self): + diff = self.ogr_project.get_pr_files_diff(7, retries=1, wait_seconds=0.1) assert isinstance(diff, dict) - assert diff == {} + assert "README.md" in diff