retries: enable retries on bad status codes

In addition to connection errors, urllib3.util.Retry will retry *only* for
explicitly listed status codes

Change default max-retry count to 10 (was 0)
Set the same for the various `build_dci_context…` functions (was 80).

The feature in responses that allows to test retries only exists in
versions for python >= 3.7, run this test only for python3.9.

Change-Id: I71c686a945eafefeb9b7e5ae1a5f97bcdb66811a
Depends-On: https://softwarefactory-project.io/r/c/dci-control-server/+/32315

Author: fcharlier

dciclient/v1/api/context.py

@@ -23,7 +23,7 @@
 import requests
 from requests.adapters import HTTPAdapter
 from requests.auth import AuthBase
-from requests.packages.urllib3.util.retry import Retry
+from urllib3.util.retry import Retry
 
 from dciauth.v2.headers import generate_headers
 from dciclient import version
@@ -32,7 +32,7 @@
 class DciContextBase(object):
     API_VERSION = "api/v1"
 
-    def __init__(self, dci_cs_url, max_retries=0, user_agent=None):
+    def __init__(self, dci_cs_url, max_retries=10, user_agent=None):
         self.session = self._build_http_session(user_agent, max_retries)
         self.dci_cs_api = "%s/%s" % (dci_cs_url, DciContext.API_VERSION)
         self.last_job_id = None
@@ -45,15 +45,20 @@ def _build_http_session(user_agent, max_retries):
             user_agent = "python-dciclient_%s" % version.__version__
         session.headers["User-Agent"] = user_agent
         session.headers["Client-Version"] = "python-dciclient_%s" % version.__version__
-        retries = Retry(total=max_retries, backoff_factor=0.1)
+        retries = Retry(
+            total=max_retries,
+            backoff_factor=0.2,
+            status_forcelist=(413, 429, 500, 502, 503, 504),
+            raise_on_status=False,
+        )
         session.mount("http://", HTTPAdapter(max_retries=retries))
         session.mount("https://", HTTPAdapter(max_retries=retries))
 
         return session
 
 
 class DciContext(DciContextBase):
-    def __init__(self, dci_cs_url, login, password, max_retries=0, user_agent=None):
+    def __init__(self, dci_cs_url, login, password, max_retries=10, user_agent=None):
         super(DciContext, self).__init__(
             dci_cs_url.rstrip("/"), max_retries, user_agent
         )
@@ -62,7 +67,7 @@ def __init__(self, dci_cs_url, login, password, max_retries=0, user_agent=None):
 
 
 def build_dci_context(
-    dci_cs_url=None, dci_login=None, dci_password=None, user_agent=None, max_retries=80
+    dci_cs_url=None, dci_login=None, dci_password=None, user_agent=None, max_retries=10
 ):
     dci_cs_url = dci_cs_url or os.environ.get("DCI_CS_URL", "")
     dci_login = dci_login or os.environ.get("DCI_LOGIN", "")
@@ -126,7 +131,7 @@ def get_body(self, body):
 
 class DciSignatureContext(DciContextBase):
     def __init__(
-        self, dci_cs_url, client_id, api_secret, max_retries=0, user_agent=None
+        self, dci_cs_url, client_id, api_secret, max_retries=10, user_agent=None
     ):
         super(DciSignatureContext, self).__init__(
             dci_cs_url.rstrip("/"), max_retries, user_agent
@@ -139,7 +144,7 @@ def build_signature_context(
     dci_client_id=None,
     dci_api_secret=None,
     user_agent=None,
-    max_retries=80,
+    max_retries=10,
 ):
     dci_cs_url = dci_cs_url or os.environ.get("DCI_CS_URL", "")
     dci_client_id = dci_client_id or os.environ.get("DCI_CLIENT_ID", "")
@@ -161,7 +166,7 @@ def build_signature_context(
 
 
 class SsoContext(DciContextBase):
-    def __init__(self, dci_cs_url, token, max_retries=0, user_agent=None):
+    def __init__(self, dci_cs_url, token, max_retries=10, user_agent=None):
         super(SsoContext, self).__init__(
             dci_cs_url.rstrip("/"), max_retries, user_agent
         )
@@ -174,7 +179,7 @@ def build_sso_context(
     username,
     password,
     token,
-    max_retries=0,
+    max_retries=10,
     user_agent=None,
     refresh=False,
 ):

test-requirements.txt

@@ -3,3 +3,4 @@ pytest
 mock
 boto3
 ansible==2.9.27
+responses

tests/shell_commands/test_cli.py

@@ -15,27 +15,24 @@
 # under the License.
 
 import os
-from mock import patch
 from pytest import raises
 from dciclient.v1.shell_commands.cli import parse_arguments
 from dciclient.v1.shell_commands.context import _default_dci_cs_url
 from dciclient.version import __version__
 
 
 def test_parse_arguments_version(capsys):
-    try:
+    with raises(SystemExit):
         parse_arguments(["--version"])
-    except SystemExit:
-        captured = capsys.readouterr()
-        assert captured.out == "dcictl {}\n".format(__version__)
+    captured = capsys.readouterr()
+    assert captured.out == "dcictl {}\n".format(__version__)
 
 
-@patch("sys.exit")
-def test_parse_no_command_print_help(exit_function, capsys):
-    parse_arguments([])
+def test_parse_no_command_print_help(capsys):
+    with raises(SystemExit):
+        parse_arguments([])
     captured = capsys.readouterr()
     assert "usage: dcictl" in captured.out
-    assert exit_function.called
 
 
 def test_parse_arguments_format():
@@ -136,9 +133,8 @@ def test_parse_arguments_dci_cs_url_overload_from_env():
 
 
 # fear test
-@patch("sys.exit")
-def test_parse_arguments_user_create_mutually_exclusive_boolean_flags(exit_function):
-    with raises(TypeError):
+def test_parse_arguments_user_create_mutually_exclusive_boolean_flags():
+    with raises(SystemExit):
         parse_arguments(
             [
                 "user-create",
@@ -152,7 +148,6 @@ def test_parse_arguments_user_create_mutually_exclusive_boolean_flags(exit_funct
                 "--no-active",
             ]
         )
-        assert exit_function.called
 
 
 def test_parse_arguments_sso():

tests/test_context.py

@@ -0,0 +1,45 @@
+# -*- encoding: utf-8 -*-
+#
+# Copyright 2024 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import sys
+
+import pytest
+import responses
+
+from dciclient.v1.api import context as dci_context
+
+
+@responses.activate
+def test_context_calls_once_on_status_ok():
+    api_ctx = dci_context.DciContextBase(dci_cs_url="http://dciapi", max_retries=4)
+    responses.add(method=responses.GET, url=api_ctx.dci_cs_api, status=200)
+
+    r = api_ctx.session.get(api_ctx.dci_cs_api)
+    assert r.status_code == 200
+    assert responses.assert_call_count(api_ctx.dci_cs_api, 1)
+
+
+@pytest.mark.xfail(
+    sys.version_info < (3, 9), reason="This test is expected to fail before python 3.8"
+)
+@responses.activate
+def test_context_retries_on_status_bad():
+    api_ctx = dci_context.DciContextBase(dci_cs_url="http://dci/api/v1")
+    responses.add(method=responses.GET, url="http://dci/api/v1", status=500)
+
+    r = api_ctx.session.get("http://dci/api/v1")
+    assert r.status_code == 500
+    assert responses.assert_call_count("http://dci/api/v1", 11)

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 skipsdist = True
-envlist = pep8,py36
+envlist = pep8,py36,py39
 
 [testenv]
 usedevelop = True
@@ -21,6 +21,11 @@ commands =
     {[testenv]commands}
     py.test -v {posargs}
 
+[testenv:py39]
+commands =
+    {[testenv]commands}
+    py.test -v {posargs}
+
 [testenv:pep8]
 skip_install = true
 deps = flake8