From d861bd4b0d4f7748439144d8b10d608a52032a54 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Thu, 21 Mar 2024 15:04:46 -0500
Subject: [PATCH 1/9] Add SessionWithMaxConnectionAge

---
 .../automation/tests/appnexus/test_base.py    | 32 +++++++++++++++++++
 .../automation/utils/appnexus/base.py         | 22 +++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/contentcuration/automation/tests/appnexus/test_base.py b/contentcuration/automation/tests/appnexus/test_base.py
index 7944e00e4f..759f8ade7c 100644
--- a/contentcuration/automation/tests/appnexus/test_base.py
+++ b/contentcuration/automation/tests/appnexus/test_base.py
@@ -1,7 +1,11 @@
+import time
 import pytest
+import requests
+from unittest.mock import patch
 
 from automation.utils.appnexus.base import Adapter
 from automation.utils.appnexus.base import Backend
+from automation.utils.appnexus.base import SessionWithMaxConnectionAge
 
 
 class MockBackend(Backend):
@@ -46,3 +50,31 @@ def test_adapter_backend_custom():
     b = MockBackend()
     a = Adapter(backend=b)
     assert a.backend is b
+
+def test_session_with_max_connection_age_request():
+    with patch.object(requests.Session, 'request') as mock_request:
+        session = SessionWithMaxConnectionAge()
+        session.request('GET', 'https://example.com')
+        assert mock_request.call_count == 1
+
+def test_session_with_max_connection_age_not_closing_connections():
+    with patch.object(requests.Session, 'close') as mock_close,\
+        patch.object(requests.Session, 'request') as mock_request:
+        session = SessionWithMaxConnectionAge(60)
+        session.request('GET', 'https://example.com')
+        time.sleep(0.1)
+        session.request('GET', 'https://example.com')
+
+        assert mock_close.call_count == 0
+        assert mock_request.call_count == 2
+
+def test_session_with_max_connection_age_closing_connections():
+    with patch.object(requests.Session, 'close') as mock_close,\
+        patch.object(requests.Session, 'request') as mock_request:
+        session = SessionWithMaxConnectionAge(1)
+        session.request('GET', 'https://example.com')
+        time.sleep(2)
+        session.request('GET', 'https://example.com')
+
+        assert mock_close.call_count == 1
+        assert mock_request.call_count == 2
\ No newline at end of file
diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index ab9e6d5096..e53af57292 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -1,8 +1,30 @@
+import time
+import requests
 from abc import ABC
 from abc import abstractmethod
 from builtins import NotImplementedError
 
 
+class SessionWithMaxConnectionAge(requests.Session):
+    """
+        Session with a maximum connection age. If the connection is older than the specified age, it will be closed and a new one will be created.
+        The age is specified in seconds.
+    """
+    def __init__(self, age = 10):
+        self.age = age
+        self.last_used = time.time()
+        super().__init__()
+
+    def request(self, *args, **kwargs):
+        current_time = time.time()
+        if current_time - self.last_used > self.age:
+            self.close()
+            self.__init__(self.age)
+
+        self.last_used = current_time
+
+        return super().request(*args, **kwargs)
+
 class BackendRequest(object):
     """ Class that should be inherited by specific backend for its requests"""
     pass

From 89e314e62bd55d954dbd5acc2695c6e00d64a0cf Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Mon, 1 Apr 2024 08:48:17 -0500
Subject: [PATCH 2/9] Base implementation

---
 .../automation/utils/appnexus/base.py         | 95 ++++++++++++++++++-
 .../automation/utils/appnexus/errors.py       | 15 +++
 2 files changed, 108 insertions(+), 2 deletions(-)
 create mode 100644 contentcuration/automation/utils/appnexus/errors.py

diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index e53af57292..5b4eedb09c 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -1,9 +1,12 @@
 import time
+import logging
 import requests
 from abc import ABC
 from abc import abstractmethod
 from builtins import NotImplementedError
 
+from . import errors
+
 
 class SessionWithMaxConnectionAge(requests.Session):
     """
@@ -32,18 +35,106 @@ class BackendRequest(object):
 
 class BackendResponse(object):
     """ Class that should be inherited by specific backend for its responses"""
-    pass
+    def __init__(self, **kwargs):
+        for key, value in kwargs.items():
+            setattr(self, key, value)
 
 
 class Backend(ABC):
     """ An abstract base class for backend interfaces that also implements the singleton pattern """
     _instance = None
+    session = None
+    base_url = None
+    connect_endpoint = None
 
-    def __new__(class_, *args, **kwargs):
+    def __new__(class_, *args, url_prefix="", **kwargs):
         if not isinstance(class_._instance, class_):
             class_._instance = object.__new__(class_, *args, **kwargs)
+            class_._instance.url_prefix = url_prefix
         return class_._instance
 
+    def __init__(self):
+        self.session = SessionWithMaxConnectionAge()
+
+    def _construct_full_url(self, path):
+        """This method should combine base_url, url_prefix, and path in that order, removing any trailing slashes beforehand."""
+        url_array = []
+        if self.base_url:
+            url_array.append(self.base_url.rstrip("/"))
+        if self.url_prefix:
+            url_array.append(self.url_prefix.rstrip("/"))
+        if path:
+            url_array.append(path.lstrip("/"))
+        return "/".join(url_array)
+
+    def _make_request(self, path, **kwargs):
+        url = self._construct_full_url(path)
+        is_retry = kwargs.pop("is_retry", False)
+        try: 
+            return self.session.request(url, **kwargs)
+        except (
+            requests.exceptions.ConnectionError,
+            requests.exceptions.RequestException,
+        ) as e:
+            if is_retry:
+                logging.error(str(e))
+                raise errors.ConnectionError("Connection error occurred. Retried request and failed again.")
+            logging.warning(f"Connection error occurred. Retrying request to {url}")
+            return self.make_request(path, is_retry=True, **kwargs)
+        except (
+            requests.exceptions.SSLError,
+        ) as e:
+            logging.error(str(e))
+            raise errors.ConnectionError(f"Unable to connect to {url}")
+        except (
+            requests.exceptions.Timeout,
+            requests.exceptions.ConnectTimeout,
+            requests.exceptions.ReadTimeout,
+        ) as e:
+            logging.error(str(e))
+            raise errors.TimeoutError(f"Timeout occurred while connecting to {url}")
+        except (
+            requests.exceptions.TooManyRedirects,
+            requests.exceptions.HTTPError,
+        ) as e:
+            logging.error(str(e))
+            raise errors.HttpError(f"HTTP error occurred while connecting to {url}")
+        except (
+            requests.exceptions.URLRequired,
+            requests.exceptions.MissingSchema,
+            requests.exceptions.InvalidSchema,
+            requests.exceptions.InvalidURL,
+            requests.exceptions.InvalidHeader,
+            requests.exceptions.InvalidJSONError,
+        ) as e:
+            logging.error(str(e))
+            raise errors.InvalidRequest(f"Invalid request to {url}")
+        except (
+            requests.exceptions.ContentDecodingError,
+            requests.exceptions.ChunkedEncodingError,
+        ) as e:
+            logging.error(str(e))
+            raise errors.InvalidResponse(f"Invalid response from {url}")
+    
+    def connect(self):
+        """ Establishes a connection to the backend service. """
+        try:
+            response = self.make_request(self.connect_endpoint, method="GET")
+            if response.status_code != 200:
+                return False
+            return True
+        except Exception as e:
+            return False
+
+    def make_request(self, path, **kwargs):
+        response = self._make_request(path, **kwargs)
+        try:
+            info = response.json()
+            return BackendResponse(**info)
+        except ValueError as e:
+            logging.error(str(e))
+            raise errors.InvalidResponse("Invalid response from backend")
+
     @abstractmethod
     def connect(self) -> None:
         """ Establishes a connection to the backend service. """
diff --git a/contentcuration/automation/utils/appnexus/errors.py b/contentcuration/automation/utils/appnexus/errors.py
new file mode 100644
index 0000000000..f9166678ab
--- /dev/null
+++ b/contentcuration/automation/utils/appnexus/errors.py
@@ -0,0 +1,15 @@
+
+class ConnectionError(Exception):
+  pass
+
+class TimeoutError(Exception):
+  pass
+
+class HttpError(Exception):
+  pass
+
+class InvalidRequest(Exception):
+  pass
+
+class InvalidResponse(Exception):
+  pass

From f4acbd521c202abbaf56df3300aa0234ed64f382 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Mon, 1 Apr 2024 11:02:06 -0500
Subject: [PATCH 3/9] Add unit tests

---
 .../automation/tests/appnexus/test_base.py    | 54 +++++++++++++++-
 .../automation/utils/appnexus/base.py         | 63 +++++++++++--------
 2 files changed, 88 insertions(+), 29 deletions(-)

diff --git a/contentcuration/automation/tests/appnexus/test_base.py b/contentcuration/automation/tests/appnexus/test_base.py
index 759f8ade7c..d6df0f1aed 100644
--- a/contentcuration/automation/tests/appnexus/test_base.py
+++ b/contentcuration/automation/tests/appnexus/test_base.py
@@ -5,10 +5,14 @@
 
 from automation.utils.appnexus.base import Adapter
 from automation.utils.appnexus.base import Backend
+from automation.utils.appnexus.base import BackendRequest
 from automation.utils.appnexus.base import SessionWithMaxConnectionAge
+from automation.utils.appnexus.errors import ConnectionError
 
 
 class MockBackend(Backend):
+    base_url = 'https://kolibri-dev.learningequality.org'
+    connect_endpoint = '/status'
     def connect(self) -> None:
         return super().connect()
 
@@ -16,8 +20,21 @@ def make_request(self, request):
         return super().make_request(request)
 
     @classmethod
-    def _create_instance(cls) -> 'MockBackend':
-        return cls()
+    def _create_instance(cls, url_prefix="") -> 'MockBackend':
+        return cls(url_prefix=url_prefix)
+
+class ErrorBackend(Backend):
+    base_url = 'https://bad-url.com'
+    connect_endpoint = '/status'
+    def connect(self) -> None:
+        return super().connect()
+    
+    def make_request(self, request):
+        return super().make_request(request)
+    
+    @classmethod
+    def _create_instance(cls, url_prefix="") -> 'ErrorBackend':
+        return cls(url_prefix=url_prefix)
 
 
 class MockAdapter(Adapter):
@@ -77,4 +94,35 @@ def test_session_with_max_connection_age_closing_connections():
         session.request('GET', 'https://example.com')
 
         assert mock_close.call_count == 1
-        assert mock_request.call_count == 2
\ No newline at end of file
+        assert mock_request.call_count == 2
+
+def test_backend_connect():
+    backend = MockBackend.get_instance()
+    connected = backend.connect()
+
+    assert connected is True
+
+def test_backend_connect_error():
+    backend = ErrorBackend.get_instance()
+    connected = backend.connect()
+
+    assert connected is False
+
+def test_backend_request():
+    request = BackendRequest('GET', '/api/public/info')
+
+    backend = MockBackend.get_instance()
+    response = backend.make_request(request)
+
+    assert response.status_code == 200
+    assert len(response.__dict__) > 0
+
+def test_backend_request_error():
+    request = BackendRequest('GET', '/api/public/info')
+
+    backend = ErrorBackend.get_instance()
+
+    with pytest.raises(ConnectionError) as error:
+        backend.make_request(request)
+    
+    assert "Connection error occurred." in str(error.value)
\ No newline at end of file
diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index 5b4eedb09c..9a9ae13069 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -29,8 +29,17 @@ def request(self, *args, **kwargs):
         return super().request(*args, **kwargs)
 
 class BackendRequest(object):
-    """ Class that should be inherited by specific backend for its requests"""
-    pass
+    def __init__(self, method, path, params=None, data=None, json=None, headers=None, max_retries=1, **kwargs):
+        self.method = method
+        self.path = path
+        self.params = params
+        self.data = data
+        self.json = json
+        self.headers = headers
+        self.max_retries = max_retries
+        self.tried = 0
+        for key, value in kwargs.items():
+            setattr(self, key, value)
 
 
 class BackendResponse(object):
@@ -47,14 +56,15 @@ class Backend(ABC):
     base_url = None
     connect_endpoint = None
 
-    def __new__(class_, *args, url_prefix="", **kwargs):
+    def __new__(class_, url_prefix="", *args, **kwargs):
         if not isinstance(class_._instance, class_):
             class_._instance = object.__new__(class_, *args, **kwargs)
             class_._instance.url_prefix = url_prefix
         return class_._instance
 
-    def __init__(self):
+    def __init__(self, url_prefix=""):
         self.session = SessionWithMaxConnectionAge()
+        self.url_prefix = url_prefix
 
     def _construct_full_url(self, path):
         """This method should combine base_url, url_prefix, and path in that order, removing any trailing slashes beforehand."""
@@ -62,25 +72,32 @@ def _construct_full_url(self, path):
         if self.base_url:
             url_array.append(self.base_url.rstrip("/"))
         if self.url_prefix:
-            url_array.append(self.url_prefix.rstrip("/"))
+            url_array.append(self.url_prefix.rstrip("/").lstrip("/"))
         if path:
             url_array.append(path.lstrip("/"))
         return "/".join(url_array)
 
-    def _make_request(self, path, **kwargs):
-        url = self._construct_full_url(path)
-        is_retry = kwargs.pop("is_retry", False)
-        try: 
-            return self.session.request(url, **kwargs)
+    def _make_request(self, request):
+        url = self._construct_full_url(request.path)
+        try:
+            request.tried += 1
+            return self.session.request(
+                request.method,
+                url,
+                params=request.params,
+                data=request.data,
+                headers=request.headers,
+                json=request.json,
+            )
         except (
             requests.exceptions.ConnectionError,
             requests.exceptions.RequestException,
         ) as e:
-            if is_retry:
+            if request.tried >= request.max_retries:
                 logging.error(str(e))
-                raise errors.ConnectionError("Connection error occurred. Retried request and failed again.")
+                raise errors.ConnectionError("Connection error occurred.")
             logging.warning(f"Connection error occurred. Retrying request to {url}")
-            return self.make_request(path, is_retry=True, **kwargs)
+            return self._make_request(request)
         except (
             requests.exceptions.SSLError,
         ) as e:
@@ -116,39 +133,33 @@ def _make_request(self, path, **kwargs):
             logging.error(str(e))
             raise errors.InvalidResponse(f"Invalid response from {url}")
     
+    @abstractmethod
     def connect(self):
         """ Establishes a connection to the backend service. """
         try:
-            response = self.make_request(self.connect_endpoint, method="GET")
+            request = BackendRequest(method="GET", path=self.connect_endpoint)
+            response = self._make_request(request)
             if response.status_code != 200:
                 return False
             return True
         except Exception as e:
             return False
 
+    @abstractmethod
     def make_request(self, path, **kwargs):
         response = self._make_request(path, **kwargs)
         try:
             info = response.json()
+            info.update({"status_code": response.status_code})
             return BackendResponse(**info)
         except ValueError as e:
             logging.error(str(e))
             raise errors.InvalidResponse("Invalid response from backend")
 
-    @abstractmethod
-    def connect(self) -> None:
-        """ Establishes a connection to the backend service. """
-        pass
-
-    @abstractmethod
-    def make_request(self, request) -> BackendResponse:
-        """ Make a request based on "request" """
-        pass
-
     @classmethod
-    def get_instance(cls) -> 'Backend':
+    def get_instance(cls, *args, **kargs) -> 'Backend':
         """ Returns existing instance, if not then create one. """
-        return cls._instance if cls._instance else cls._create_instance()
+        return cls._instance if cls._instance else cls._create_instance(*args, **kargs)
 
     @classmethod
     def _create_instance(cls) -> 'Backend':

From 064a5034e63d5b83c9a1108867a950c8a6319c27 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Thu, 4 Apr 2024 08:27:02 -0500
Subject: [PATCH 4/9] Refactor singleton

---
 .../automation/tests/appnexus/test_base.py    | 23 ++++---------------
 .../automation/utils/appnexus/base.py         | 22 ++++--------------
 2 files changed, 10 insertions(+), 35 deletions(-)

diff --git a/contentcuration/automation/tests/appnexus/test_base.py b/contentcuration/automation/tests/appnexus/test_base.py
index d6df0f1aed..552ec5be8c 100644
--- a/contentcuration/automation/tests/appnexus/test_base.py
+++ b/contentcuration/automation/tests/appnexus/test_base.py
@@ -19,10 +19,6 @@ def connect(self) -> None:
     def make_request(self, request):
         return super().make_request(request)
 
-    @classmethod
-    def _create_instance(cls, url_prefix="") -> 'MockBackend':
-        return cls(url_prefix=url_prefix)
-
 class ErrorBackend(Backend):
     base_url = 'https://bad-url.com'
     connect_endpoint = '/status'
@@ -31,10 +27,6 @@ def connect(self) -> None:
     
     def make_request(self, request):
         return super().make_request(request)
-    
-    @classmethod
-    def _create_instance(cls, url_prefix="") -> 'ErrorBackend':
-        return cls(url_prefix=url_prefix)
 
 
 class MockAdapter(Adapter):
@@ -42,13 +34,8 @@ def mockoperation(self):
         pass
 
 
-def test_backend_error():
-    with pytest.raises(NotImplementedError) as error:
-        Backend.get_instance()
-    assert "Subclasses should implement the creation of instance" in str(error.value)
-
 def test_backend_singleton():
-    b1, b2 = MockBackend.get_instance(), MockBackend.get_instance()
+    b1, b2 = MockBackend(), MockBackend()
     assert id(b1) == id(b2)
 
 
@@ -97,13 +84,13 @@ def test_session_with_max_connection_age_closing_connections():
         assert mock_request.call_count == 2
 
 def test_backend_connect():
-    backend = MockBackend.get_instance()
+    backend = MockBackend()
     connected = backend.connect()
 
     assert connected is True
 
 def test_backend_connect_error():
-    backend = ErrorBackend.get_instance()
+    backend = ErrorBackend()
     connected = backend.connect()
 
     assert connected is False
@@ -111,7 +98,7 @@ def test_backend_connect_error():
 def test_backend_request():
     request = BackendRequest('GET', '/api/public/info')
 
-    backend = MockBackend.get_instance()
+    backend = MockBackend()
     response = backend.make_request(request)
 
     assert response.status_code == 200
@@ -120,7 +107,7 @@ def test_backend_request():
 def test_backend_request_error():
     request = BackendRequest('GET', '/api/public/info')
 
-    backend = ErrorBackend.get_instance()
+    backend = ErrorBackend()
 
     with pytest.raises(ConnectionError) as error:
         backend.make_request(request)
diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index 9a9ae13069..2143807c75 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -14,15 +14,14 @@ class SessionWithMaxConnectionAge(requests.Session):
         The age is specified in seconds.
     """
     def __init__(self, age = 10):
+        super().__init__()
         self.age = age
         self.last_used = time.time()
-        super().__init__()
 
     def request(self, *args, **kwargs):
         current_time = time.time()
         if current_time - self.last_used > self.age:
             self.close()
-            self.__init__(self.age)
 
         self.last_used = current_time
 
@@ -56,11 +55,10 @@ class Backend(ABC):
     base_url = None
     connect_endpoint = None
 
-    def __new__(class_, url_prefix="", *args, **kwargs):
-        if not isinstance(class_._instance, class_):
-            class_._instance = object.__new__(class_, *args, **kwargs)
-            class_._instance.url_prefix = url_prefix
-        return class_._instance
+    def __new__(cls, *args, **kwargs):
+        if not isinstance(cls._instance, cls):
+            cls._instance = object.__new__(cls)
+        return cls._instance
 
     def __init__(self, url_prefix=""):
         self.session = SessionWithMaxConnectionAge()
@@ -156,16 +154,6 @@ def make_request(self, path, **kwargs):
             logging.error(str(e))
             raise errors.InvalidResponse("Invalid response from backend")
 
-    @classmethod
-    def get_instance(cls, *args, **kargs) -> 'Backend':
-        """ Returns existing instance, if not then create one. """
-        return cls._instance if cls._instance else cls._create_instance(*args, **kargs)
-
-    @classmethod
-    def _create_instance(cls) -> 'Backend':
-        """ Returns the instance after creating it. """
-        raise NotImplementedError("Subclasses should implement the creation of instance")
-
 
 class BackendFactory(ABC):
     @abstractmethod

From 267935c1c89d4475ed49399f8ec37c4175ca74fa Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Thu, 4 Apr 2024 08:55:32 -0500
Subject: [PATCH 5/9] Refactor retry strategy

---
 .../automation/tests/appnexus/test_base.py    |  2 +-
 .../automation/utils/appnexus/base.py         | 62 +++++++++++++------
 2 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/contentcuration/automation/tests/appnexus/test_base.py b/contentcuration/automation/tests/appnexus/test_base.py
index 552ec5be8c..b06cc213b5 100644
--- a/contentcuration/automation/tests/appnexus/test_base.py
+++ b/contentcuration/automation/tests/appnexus/test_base.py
@@ -112,4 +112,4 @@ def test_backend_request_error():
     with pytest.raises(ConnectionError) as error:
         backend.make_request(request)
     
-    assert "Connection error occurred." in str(error.value)
\ No newline at end of file
+    assert "Unable to connect to" in str(error.value)
diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index 2143807c75..deffb0d6ba 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -4,6 +4,8 @@
 from abc import ABC
 from abc import abstractmethod
 from builtins import NotImplementedError
+from requests.adapters import HTTPAdapter
+from requests.packages.urllib3.util.retry import Retry
 
 from . import errors
 
@@ -27,16 +29,25 @@ def request(self, *args, **kwargs):
 
         return super().request(*args, **kwargs)
 
+
 class BackendRequest(object):
-    def __init__(self, method, path, params=None, data=None, json=None, headers=None, max_retries=1, **kwargs):
+    """ Class that holds the request information for the backend """
+    def __init__(
+        self,
+        method,
+        path,
+        params=None,
+        data=None,
+        json=None,
+        headers=None,
+        **kwargs
+    ):
         self.method = method
         self.path = path
         self.params = params
         self.data = data
         self.json = json
         self.headers = headers
-        self.max_retries = max_retries
-        self.tried = 0
         for key, value in kwargs.items():
             setattr(self, key, value)
 
@@ -54,18 +65,36 @@ class Backend(ABC):
     session = None
     base_url = None
     connect_endpoint = None
+    max_retries=1
+    backoff_factor=0.3
 
     def __new__(cls, *args, **kwargs):
         if not isinstance(cls._instance, cls):
             cls._instance = object.__new__(cls)
         return cls._instance
 
-    def __init__(self, url_prefix=""):
-        self.session = SessionWithMaxConnectionAge()
+    def __init__(
+        self,
+        url_prefix="",
+    ):
         self.url_prefix = url_prefix
+        if not self.session:
+            self._setup_session()
+
+    def _setup_session(self):
+        self.session = SessionWithMaxConnectionAge()
+
+        retry = Retry(
+            total=self.max_retries,
+            backoff_factor=self.backoff_factor,
+        )
+        adapter = HTTPAdapter(max_retries=retry)
+
+        self.session.mount("https://", adapter)
+        self.session.mount("http://", adapter)
 
     def _construct_full_url(self, path):
-        """This method should combine base_url, url_prefix, and path in that order, removing any trailing slashes beforehand."""
+        """This method combine base_url, url_prefix, and path in that order, removing any trailing and leading slashes."""
         url_array = []
         if self.base_url:
             url_array.append(self.base_url.rstrip("/"))
@@ -78,7 +107,6 @@ def _construct_full_url(self, path):
     def _make_request(self, request):
         url = self._construct_full_url(request.path)
         try:
-            request.tried += 1
             return self.session.request(
                 request.method,
                 url,
@@ -90,29 +118,22 @@ def _make_request(self, request):
         except (
             requests.exceptions.ConnectionError,
             requests.exceptions.RequestException,
-        ) as e:
-            if request.tried >= request.max_retries:
-                logging.error(str(e))
-                raise errors.ConnectionError("Connection error occurred.")
-            logging.warning(f"Connection error occurred. Retrying request to {url}")
-            return self._make_request(request)
-        except (
             requests.exceptions.SSLError,
         ) as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.ConnectionError(f"Unable to connect to {url}")
         except (
             requests.exceptions.Timeout,
             requests.exceptions.ConnectTimeout,
             requests.exceptions.ReadTimeout,
         ) as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.TimeoutError(f"Timeout occurred while connecting to {url}")
         except (
             requests.exceptions.TooManyRedirects,
             requests.exceptions.HTTPError,
         ) as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.HttpError(f"HTTP error occurred while connecting to {url}")
         except (
             requests.exceptions.URLRequired,
@@ -122,13 +143,13 @@ def _make_request(self, request):
             requests.exceptions.InvalidHeader,
             requests.exceptions.InvalidJSONError,
         ) as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.InvalidRequest(f"Invalid request to {url}")
         except (
             requests.exceptions.ContentDecodingError,
             requests.exceptions.ChunkedEncodingError,
         ) as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.InvalidResponse(f"Invalid response from {url}")
     
     @abstractmethod
@@ -145,13 +166,14 @@ def connect(self):
 
     @abstractmethod
     def make_request(self, path, **kwargs):
+        """ Make a request to the backend service. """
         response = self._make_request(path, **kwargs)
         try:
             info = response.json()
             info.update({"status_code": response.status_code})
             return BackendResponse(**info)
         except ValueError as e:
-            logging.error(str(e))
+            logging.exception(e)
             raise errors.InvalidResponse("Invalid response from backend")
 
 

From 2d356dcd3f22cbfe7e5fc9761ba9bccbacdacd44 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Tue, 9 Apr 2024 10:51:42 -0500
Subject: [PATCH 6/9] Refactor tests

---
 .../contentcuration/tests/utils/test_cloud_storage.py           | 2 +-
 .../contentcuration/tests/utils/test_recommendations.py         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contentcuration/contentcuration/tests/utils/test_cloud_storage.py b/contentcuration/contentcuration/tests/utils/test_cloud_storage.py
index 3aade0d72a..5d84fd9f10 100644
--- a/contentcuration/contentcuration/tests/utils/test_cloud_storage.py
+++ b/contentcuration/contentcuration/tests/utils/test_cloud_storage.py
@@ -7,4 +7,4 @@ class CloudStorageTestCase(TestCase):
     def test_backend_initialization(self):
         cloud_storage_instance = CloudStorage()
         self.assertIsNotNone(cloud_storage_instance)
-        self.assertIsInstance(cloud_storage_instance.get_instance(), CloudStorage)
+        self.assertIsInstance(cloud_storage_instance, CloudStorage)
diff --git a/contentcuration/contentcuration/tests/utils/test_recommendations.py b/contentcuration/contentcuration/tests/utils/test_recommendations.py
index d410651de1..a4f711033a 100644
--- a/contentcuration/contentcuration/tests/utils/test_recommendations.py
+++ b/contentcuration/contentcuration/tests/utils/test_recommendations.py
@@ -7,4 +7,4 @@ class RecommendationsTestCase(TestCase):
     def test_backend_initialization(self):
         recomendations = Recommendations()
         self.assertIsNotNone(recomendations)
-        self.assertIsInstance(recomendations.get_instance(), Recommendations)
+        self.assertIsInstance(recomendations, Recommendations)

From 5877432e3e6f009cf7044401c75e63f6b7cba628 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Mon, 6 May 2024 09:51:26 -0500
Subject: [PATCH 7/9] Add timeout to request

---
 contentcuration/automation/utils/appnexus/base.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index deffb0d6ba..9341a00f6e 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -40,6 +40,7 @@ def __init__(
         data=None,
         json=None,
         headers=None,
+        timeout=(5, 10),
         **kwargs
     ):
         self.method = method
@@ -48,6 +49,7 @@ def __init__(
         self.data = data
         self.json = json
         self.headers = headers
+        self.timeout = timeout
         for key, value in kwargs.items():
             setattr(self, key, value)
 
@@ -114,6 +116,7 @@ def _make_request(self, request):
                 data=request.data,
                 headers=request.headers,
                 json=request.json,
+                timeout=request.timeout,
             )
         except (
             requests.exceptions.ConnectionError,
@@ -153,10 +156,10 @@ def _make_request(self, request):
             raise errors.InvalidResponse(f"Invalid response from {url}")
     
     @abstractmethod
-    def connect(self):
+    def connect(self, **kwargs):
         """ Establishes a connection to the backend service. """
         try:
-            request = BackendRequest(method="GET", path=self.connect_endpoint)
+            request = BackendRequest(method="GET", path=self.connect_endpoint, **kwargs)
             response = self._make_request(request)
             if response.status_code != 200:
                 return False
@@ -165,9 +168,9 @@ def connect(self):
             return False
 
     @abstractmethod
-    def make_request(self, path, **kwargs):
+    def make_request(self, request):
         """ Make a request to the backend service. """
-        response = self._make_request(path, **kwargs)
+        response = self._make_request(request)
         try:
             info = response.json()
             info.update({"status_code": response.status_code})

From a65f83194d2b8051233d88e7489168fc3e31095b Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Mon, 6 May 2024 15:07:12 -0500
Subject: [PATCH 8/9] Update timeout value

---
 contentcuration/automation/utils/appnexus/base.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index 9341a00f6e..214ec9d1e1 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -40,7 +40,7 @@ def __init__(
         data=None,
         json=None,
         headers=None,
-        timeout=(5, 10),
+        timeout=(5, 100),
         **kwargs
     ):
         self.method = method

From b3861018f43f7f768cb19b200650e879de91a0b4 Mon Sep 17 00:00:00 2001
From: Alex Velez <velezllaquealex@gmail.com>
Date: Wed, 15 May 2024 14:09:55 -0500
Subject: [PATCH 9/9] Address comments

---
 contentcuration/automation/utils/appnexus/base.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/contentcuration/automation/utils/appnexus/base.py b/contentcuration/automation/utils/appnexus/base.py
index 214ec9d1e1..efbc41f100 100644
--- a/contentcuration/automation/utils/appnexus/base.py
+++ b/contentcuration/automation/utils/appnexus/base.py
@@ -15,7 +15,7 @@ class SessionWithMaxConnectionAge(requests.Session):
         Session with a maximum connection age. If the connection is older than the specified age, it will be closed and a new one will be created.
         The age is specified in seconds.
     """
-    def __init__(self, age = 10):
+    def __init__(self, age = 100):
         super().__init__()
         self.age = age
         self.last_used = time.time()
@@ -109,7 +109,7 @@ def _construct_full_url(self, path):
     def _make_request(self, request):
         url = self._construct_full_url(request.path)
         try:
-            return self.session.request(
+            response = self.session.request(
                 request.method,
                 url,
                 params=request.params,
@@ -118,6 +118,8 @@ def _make_request(self, request):
                 json=request.json,
                 timeout=request.timeout,
             )
+            response.raise_for_status()
+            return response
         except (
             requests.exceptions.ConnectionError,
             requests.exceptions.RequestException,
@@ -155,19 +157,15 @@ def _make_request(self, request):
             logging.exception(e)
             raise errors.InvalidResponse(f"Invalid response from {url}")
     
-    @abstractmethod
     def connect(self, **kwargs):
         """ Establishes a connection to the backend service. """
         try:
             request = BackendRequest(method="GET", path=self.connect_endpoint, **kwargs)
-            response = self._make_request(request)
-            if response.status_code != 200:
-                return False
+            self._make_request(request)
             return True
         except Exception as e:
             return False
 
-    @abstractmethod
     def make_request(self, request):
         """ Make a request to the backend service. """
         response = self._make_request(request)