From c47f071d01ee67161b984ace600660b3ccdc2809 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Boll <jboll@redhat.com>
Date: Thu, 1 Sep 2022 13:45:30 +0200
Subject: [PATCH] Add retries to get features (#218)

* Add retries to get features

Create session and provide it with a custom HTTPAdapter, that has max_retries set.

Related issue: https://github.com/Unleash/unleash-client-python/issues/217

* Fix Retry type

* Fix import, use urllib3.Retry instead of Sonarype suggestion
---
 UnleashClient/api/features.py        | 18 ++++++++++-----
 UnleashClient/constants.py           |  1 +
 tests/unit_tests/api/test_feature.py | 33 ++++++++++++++++++++++------
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/UnleashClient/api/features.py b/UnleashClient/api/features.py
index 267373c8..9774788a 100644
--- a/UnleashClient/api/features.py
+++ b/UnleashClient/api/features.py
@@ -1,6 +1,10 @@
 from typing import Tuple
 import requests
-from UnleashClient.constants import REQUEST_TIMEOUT, FEATURES_URL
+from requests.adapters import HTTPAdapter
+from urllib3 import Retry
+
+from UnleashClient.constants import REQUEST_TIMEOUT, FEATURES_URL, \
+    REQUEST_RETRIES
 from UnleashClient.utils import LOGGER, log_resp_info
 
 
@@ -45,10 +49,14 @@ def get_feature_toggles(url: str,
         if project:
             base_params = {'project': project}
 
-        resp = requests.get(base_url,
-                            headers={**custom_headers, **headers},
-                            params=base_params,
-                            timeout=REQUEST_TIMEOUT, **custom_options)
+        adapter = HTTPAdapter(max_retries=Retry(total=REQUEST_RETRIES, status_forcelist=[500, 502, 504]))
+        with requests.Session() as session:
+            session.mount("https://", adapter)
+            session.mount("http://", adapter)
+            resp = session.get(base_url,
+                               headers={**custom_headers, **headers},
+                               params=base_params,
+                               timeout=REQUEST_TIMEOUT, **custom_options)
 
         if resp.status_code not in [200, 304]:
             log_resp_info(resp)
diff --git a/UnleashClient/constants.py b/UnleashClient/constants.py
index 9bbc91f4..1c97b4e7 100644
--- a/UnleashClient/constants.py
+++ b/UnleashClient/constants.py
@@ -4,6 +4,7 @@
 SDK_NAME = "unleash-client-python"
 SDK_VERSION = version("UnleashClient")
 REQUEST_TIMEOUT = 30
+REQUEST_RETRIES = 3
 METRIC_LAST_SENT_TIME = "mlst"
 
 # =Unleash=
diff --git a/tests/unit_tests/api/test_feature.py b/tests/unit_tests/api/test_feature.py
index ec1e48b9..d929ceb7 100644
--- a/tests/unit_tests/api/test_feature.py
+++ b/tests/unit_tests/api/test_feature.py
@@ -1,5 +1,6 @@
 import responses
 from pytest import mark, param
+
 from tests.utilities.mocks.mock_features import MOCK_FEATURE_RESPONSE, MOCK_FEATURE_RESPONSE_PROJECT
 from tests.utilities.testing_constants import URL, APP_NAME, INSTANCE_ID, CUSTOM_HEADERS, CUSTOM_OPTIONS, PROJECT_URL, PROJECT_NAME, ETAG_VALUE
 from UnleashClient.constants import FEATURES_URL
@@ -10,12 +11,12 @@
 
 
 @responses.activate
-@mark.parametrize("response,status,expected", (
-    param(MOCK_FEATURE_RESPONSE, 200, lambda result: result["version"] == 1, id="success"),
-    param(MOCK_FEATURE_RESPONSE, 202, lambda result: not result, id="failure"),
-    param({}, 500, lambda result: not result, id="failure"),
+@mark.parametrize("response,status,calls,expected", (
+    param(MOCK_FEATURE_RESPONSE, 200, 1, lambda result: result["version"] == 1, id="success"),
+    param(MOCK_FEATURE_RESPONSE, 202, 1, lambda result: not result, id="failure"),
+    param({}, 500, 4, lambda result: not result, id="failure"),
 ))
-def test_get_feature_toggle(response, status, expected):
+def test_get_feature_toggle(response, status, calls, expected):
     responses.add(responses.GET, FULL_FEATURE_URL, json=response, status=status, headers={'etag': ETAG_VALUE})
 
     (result, etag) = get_feature_toggles(URL,
@@ -24,7 +25,7 @@ def test_get_feature_toggle(response, status, expected):
                                          CUSTOM_HEADERS,
                                          CUSTOM_OPTIONS)
 
-    assert len(responses.calls) == 1
+    assert len(responses.calls) == calls
     assert expected(result)
 
 
@@ -55,7 +56,7 @@ def test_get_feature_toggle_failed_etag():
                                          CUSTOM_OPTIONS,
                                          PROJECT_NAME)
 
-    assert len(responses.calls) == 1
+    assert len(responses.calls) == 4
     assert etag == ''
 
 
@@ -75,3 +76,21 @@ def test_get_feature_toggle_etag_present():
     assert not result
     assert responses.calls[0].request.headers['If-None-Match'] == ETAG_VALUE
     assert etag == ETAG_VALUE
+
+
+@responses.activate
+def test_get_feature_toggle_retries():
+    responses.add(responses.GET, PROJECT_URL, json={}, status=500)
+    responses.add(responses.GET, PROJECT_URL, json=MOCK_FEATURE_RESPONSE_PROJECT, status=200, headers={'etag': ETAG_VALUE})
+
+    (result, etag) = get_feature_toggles(URL,
+                                     APP_NAME,
+                                     INSTANCE_ID,
+                                     CUSTOM_HEADERS,
+                                     CUSTOM_OPTIONS,
+                                     PROJECT_NAME,
+                                     ETAG_VALUE)
+
+    assert len(responses.calls) == 2
+    assert len(result["features"]) == 1
+    assert etag == ETAG_VALUE