Fix race condition causing ENOPRSTATS (#827)

Fix race condition causing ENOPRSTATS

When we get the ENOPRSTATS exception from Pagure there is a race condition.
We can try to run the call again and see if it solves the problem.
Fixes packit/packit-service#2287
Merge before packit/packit-service#2289

Reviewed-by: Nikola Forró
Reviewed-by: Maja Massarini

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.
 

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)

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(

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

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