From 14c039ac2b148620bc1acc032e7745cb131ac18c Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 1 Jun 2022 16:57:23 -0400
Subject: [PATCH 01/26] Modify Manifester class to work with MockStub

Add a class to test_manifester.py to enable requests to be sent to a
MockStub version of the requests package instead of sending live
requests to the API. Modify the Manifester class to use the
MockStub-based requests when running unit tests. Add a helper function
to generate mock HTTP response codes.
---
 manifester/helpers.py    | 47 ++++++++++++++++++++++++++++++++++++
 manifester/manifester.py | 31 ++++++++++++++----------
 tests/test_manifester.py | 52 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+), 13 deletions(-)
 create mode 100644 tests/test_manifester.py

diff --git a/manifester/helpers.py b/manifester/helpers.py
index 08e6a26..d21ca2e 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -1,5 +1,7 @@
+import random
 import time
 
+from collections import UserDict
 from logzero import logger
 
 
@@ -12,6 +14,7 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
     # with caution as some data (notably the offline token) should be treated as a secret.
     logger.debug(f"Sending request to endpoint {cmd_args}")
     response = cmd(*cmd_args, **cmd_kwargs)
+    breakpoint()
     logger.debug(f"Response status code is {response.status_code}")
     if response.status_code in [429, 500, 504]:
         new_wait = _cur_timeout * 2
@@ -39,3 +42,47 @@ def process_sat_version(sat_version, valid_sat_versions):
         if sat_version not in valid_sat_versions:
             valid_sat_versions.sort(key = lambda i: int(i.split('-')[-1].split('.')[-1]), reverse = True)
     return valid_sat_versions[0]
+
+def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
+    # randomish = random.random()
+    # print(randomish, fail_rate/100)
+    if random.random() > (fail_rate / 100):
+        return random.choice(good_codes)
+    else:
+        return random.choice(bad_codes)
+
+
+class MockStub(UserDict):
+    """Test helper class. Allows for both arbitrary mocking and stubbing"""
+
+    def __init__(self, in_dict=None):
+        """Initialize the class and all nested dictionaries"""
+        if in_dict is None:
+            in_dict = {}
+        for key, value in in_dict.items():
+            if isinstance(value, dict):
+                setattr(self, key, MockStub(value))
+            elif type(value) in (list, tuple):
+                setattr(
+                    self,
+                    key,
+                    [MockStub(x) if isinstance(x, dict) else x for x in value],
+                )
+            else:
+                setattr(self, key, value)
+        super().__init__(in_dict)
+
+    def __getattr__(self, name):
+        return self
+
+    def __getitem__(self, key):
+        if isinstance(key, str):
+            item = getattr(self, key, self)
+        try:
+            item = super().__getitem__(key)
+        except KeyError:
+            item = self
+        return item
+
+    def __call__(self, *args, **kwargs):
+        return self
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 5cc3747..9c7935b 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -4,7 +4,6 @@
 from functools import cached_property
 from pathlib import Path
 
-import requests
 from logzero import logger
 
 from manifester.helpers import simple_retry
@@ -19,6 +18,11 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
             self.manifest_data = DynaBox(manifest_category)
         else:
             self.manifest_data = settings.manifest_category.get(manifest_category)
+        if kwargs.get("requester") is not None:
+            self.requester = kwargs["requester"]
+        else:
+            import requests
+            self.requester = requests
         self.allocation_name = allocation_name or "".join(
             random.sample(string.ascii_letters, 10)
         )
@@ -48,7 +52,7 @@ def access_token(self):
             token_request_data = {"data": self.token_request_data}
             logger.debug("Generating access token")
             token_data = simple_retry(
-                requests.post,
+                self.requester.post,
                 cmd_args=[f"{self.token_request_url}"],
                 cmd_kwargs=token_request_data,
             ).json()
@@ -63,7 +67,7 @@ def valid_sat_versions(self):
         }
         valid_sat_versions = []
         sat_versions_response = simple_retry(
-            requests.get,
+            self.requester.get,
             cmd_args=[
                     f"{self.allocations_url}/versions"
                 ],
@@ -84,7 +88,7 @@ def create_subscription_allocation(self):
             },
         }
         self.allocation = simple_retry(
-            requests.post,
+            self.requester.post,
             cmd_args=[f"{self.allocations_url}"],
             cmd_kwargs=allocation_data,
         ).json()
@@ -101,7 +105,7 @@ def create_subscription_allocation(self):
         self.allocation_uuid = self.allocation["body"]["uuid"]
         if self.simple_content_access == "disabled":
             simple_retry(
-                requests.put,
+                self.requester.put,
                 cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}"],
                 cmd_kwargs={
                     "headers": {"Authorization": f"Bearer {self.access_token}"},
@@ -139,7 +143,7 @@ def subscription_pools(self):
                 "params": {"offset": _offset},
             }
             self._subscription_pools = simple_retry(
-                requests.get,
+                self.requester.get,
                 cmd_args=[
                     f"{self.allocations_url}/{self.allocation_uuid}/pools"
                 ],
@@ -161,7 +165,7 @@ def subscription_pools(self):
                     "params": {"offset": _offset},
                 }
                 offset_pools = simple_retry(
-                    requests.get,
+                    self.requester.get,
                     cmd_args=[
                         f"{self.allocations_url}/{self.allocation_uuid}/pools"
                     ],
@@ -182,7 +186,7 @@ def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
             "params": {"pool": f"{pool_id}", "quantity": f"{entitlement_quantity}"},
         }
         add_entitlements = simple_retry(
-            requests.post,
+            self.requester.post,
             cmd_args=[
                 f"{self.allocations_url}/{self.allocation_uuid}/entitlements"
             ],
@@ -200,13 +204,14 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
             "params": {"include": "entitlements"},
         }
         self.entitlement_data = simple_retry(
-            requests.get,
+            self.requester.get,
             cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}"],
             cmd_kwargs=data,
         ).json()
         current_entitlement = [
             d
             for d in self.entitlement_data["body"]["entitlementsAttached"]["value"]
+CONFLICT (content): Merge conflict in manifester/manifester.py
             if d["subscriptionName"] == subscription_name
         ]
         if not current_entitlement:
@@ -313,7 +318,7 @@ def trigger_manifest_export(self):
             f"Triggering manifest export job for subscription allocation {self.allocation_name}"
         )
         trigger_export_job = simple_retry(
-            requests.get,
+            self.requester.get,
             cmd_args=[
                 f"{self.allocations_url}/{self.allocation_uuid}/export"
             ],
@@ -321,7 +326,7 @@ def trigger_manifest_export(self):
         ).json()
         export_job_id = trigger_export_job["body"]["exportJobID"]
         export_job = simple_retry(
-            requests.get,
+            self.requester.get,
             cmd_args=[
                 f"{self.allocations_url}/{self.allocation_uuid}/exportJob/{export_job_id}"
             ],
@@ -331,7 +336,7 @@ def trigger_manifest_export(self):
         limit_exceeded = False
         while export_job.status_code != 200:
             export_job = simple_retry(
-                requests.get,
+                self.requester.get,
                 cmd_args=[
                     f"{self.allocations_url}/{self.allocation_uuid}/exportJob/{export_job_id}"
                 ],
@@ -354,7 +359,7 @@ def trigger_manifest_export(self):
         export_job = export_job.json()
         export_href = export_job["body"]["href"]
         manifest = simple_retry(
-            requests.get,
+            self.requester.get,
             cmd_args=[f"{export_href}"],
             cmd_kwargs=data,
         )
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
new file mode 100644
index 0000000..5a69bae
--- /dev/null
+++ b/tests/test_manifester.py
@@ -0,0 +1,52 @@
+from unittest.mock import Mock
+
+from requests import request
+from manifester import Manifester
+from manifester.settings import settings
+from manifester.helpers import MockStub, fake_http_response_code
+import pytest
+import random
+
+def test_empty_init(manifest_category="golden_ticket"):
+    manifester_inst = Manifester(manifest_category=manifest_category)
+    assert isinstance(manifester_inst, Manifester)
+
+class RhsmApiStub(MockStub):
+    def __init__(self, in_dict=None, **kwargs):
+        self._good_codes = kwargs.get("good_codes", [200])
+        self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
+        self._fail_rate = kwargs.get("fail_rate", 10)
+        super().__init__(in_dict)
+
+    @property
+    def status_code(self):
+        return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
+
+    def post(*args, **kwargs):
+        if args[0].endswith("openid-connect/token"):
+            return MockStub(in_dict={"access_token": "this is a simulated access token"}, status_code=200)
+        if args[0].endswith("allocations"):
+            return MockStub(in_dict={"uuid": "1234567890"})
+        if args[0].endswith("entitlements"):
+            return MockStub(status_code=200)
+
+    def get(*args, **kwargs):
+        if args[0].endswith("pools"):
+            # question: how to fake > 50 pools to test use of offset parameter?
+            return MockStub(in_dict={"pool": "this is a simulated list of dictionaries of subscription pool data"})
+        if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
+            return MockStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
+        if args[0].endswith("export"):
+            return MockStub(in_dict={"export_job": "Manifest export job triggered successfully"})
+        if "exportJob" in args[0]:
+            responses = [202, 200]
+            return MockStub(status_code=random.choice(responses))
+        if "export" in args[0] and not args[0].endswith("export"):
+            return MockStub(in_dict={"content": "this is a simulated manifest"})
+
+
+def test_create_allocation():
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, status_code=200))
+    allocation_uuid = manifester.create_subscription_allocation()
+    breakpoint()
+    assert allocation_uuid == "1234567890"

From 0996b26b0abcefe251d6beac05f086d19cb1a6a4 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 8 Nov 2023 14:39:59 -0500
Subject: [PATCH 02/26] Rebase PR and address merge conflicts

---
 manifester/helpers.py    | 1 -
 manifester/manifester.py | 1 -
 2 files changed, 2 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index d21ca2e..f11d9fc 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -14,7 +14,6 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
     # with caution as some data (notably the offline token) should be treated as a secret.
     logger.debug(f"Sending request to endpoint {cmd_args}")
     response = cmd(*cmd_args, **cmd_kwargs)
-    breakpoint()
     logger.debug(f"Response status code is {response.status_code}")
     if response.status_code in [429, 500, 504]:
         new_wait = _cur_timeout * 2
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 9c7935b..2b79144 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -211,7 +211,6 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
         current_entitlement = [
             d
             for d in self.entitlement_data["body"]["entitlementsAttached"]["value"]
-CONFLICT (content): Merge conflict in manifester/manifester.py
             if d["subscriptionName"] == subscription_name
         ]
         if not current_entitlement:

From 36ba3441688c46cfef87ea52e382aa8aca78d539 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 14 Nov 2023 16:13:44 -0500
Subject: [PATCH 03/26] Get initial tests passing

This commit makes several modifications to the test harness, helper
functions, and manifester itself to get the first two unit tests into a
working state.
---
 manifester/helpers.py    |  3 ++-
 manifester/manifester.py |  9 +++++++--
 tests/test_manifester.py | 36 ++++++++++++++++++++----------------
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index f11d9fc..a70abb2 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -40,7 +40,8 @@ def process_sat_version(sat_version, valid_sat_versions):
         # If sat_version is still not valid, default to the latest valid version.
         if sat_version not in valid_sat_versions:
             valid_sat_versions.sort(key = lambda i: int(i.split('-')[-1].split('.')[-1]), reverse = True)
-    return valid_sat_versions[0]
+            return valid_sat_versions[0]
+    return sat_version
 
 def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
     # randomish = random.random()
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 2b79144..738dfb2 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -20,9 +20,11 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
             self.manifest_data = settings.manifest_category.get(manifest_category)
         if kwargs.get("requester") is not None:
             self.requester = kwargs["requester"]
+            self.is_mock = True
         else:
             import requests
             self.requester = requests
+            self.is_mock = False
         self.allocation_name = allocation_name or "".join(
             random.sample(string.ascii_letters, 10)
         )
@@ -73,8 +75,11 @@ def valid_sat_versions(self):
                 ],
             cmd_kwargs=headers,
             ).json()
-        for ver_dict in sat_versions_response["body"]:
-            valid_sat_versions.append(ver_dict["value"])
+        if self.is_mock is False:
+            for ver_dict in sat_versions_response["body"]:
+                valid_sat_versions.append(ver_dict["value"])
+        else:
+            valid_sat_versions = sat_versions_response["valid_sat_versions"]
         return valid_sat_versions
 
     def create_subscription_allocation(self):
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 5a69bae..bb910e7 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -8,45 +8,49 @@
 import random
 
 def test_empty_init(manifest_category="golden_ticket"):
-    manifester_inst = Manifester(manifest_category=manifest_category)
+    manifester_inst = Manifester(manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None))
+    breakpoint()
     assert isinstance(manifester_inst, Manifester)
+    assert manifester_inst.access_token == 'this is a simulated access token'
 
 class RhsmApiStub(MockStub):
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
         self._fail_rate = kwargs.get("fail_rate", 10)
+        self.status_code = fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
         super().__init__(in_dict)
 
-    @property
-    def status_code(self):
-        return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
+    # @property
+    # def status_code(self):
+    #     return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
 
     def post(*args, **kwargs):
-        if args[0].endswith("openid-connect/token"):
-            return MockStub(in_dict={"access_token": "this is a simulated access token"}, status_code=200)
-        if args[0].endswith("allocations"):
-            return MockStub(in_dict={"uuid": "1234567890"})
-        if args[0].endswith("entitlements"):
-            return MockStub(status_code=200)
+        if args[1].endswith("openid-connect/token"):
+            return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
+        if args[1].endswith("allocations/"):
+            return RhsmApiStub(in_dict={"uuid": "1234567890"})
+        if args[1].endswith("entitlements"):
+            return RhsmApiStub(status_code=200)
 
     def get(*args, **kwargs):
-        if args[0].endswith("pools"):
+        if args[1].endswith("versions"):
+            return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
+        if args[1].endswith("pools"):
             # question: how to fake > 50 pools to test use of offset parameter?
             return MockStub(in_dict={"pool": "this is a simulated list of dictionaries of subscription pool data"})
-        if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
+        if "allocations" in args[1] and not ("export" in args[1] or "pools" in args[1]):
             return MockStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
-        if args[0].endswith("export"):
+        if args[1].endswith("export"):
             return MockStub(in_dict={"export_job": "Manifest export job triggered successfully"})
-        if "exportJob" in args[0]:
+        if "exportJob" in args[1]:
             responses = [202, 200]
             return MockStub(status_code=random.choice(responses))
-        if "export" in args[0] and not args[0].endswith("export"):
+        if "export" in args[1] and not args[1].endswith("export"):
             return MockStub(in_dict={"content": "this is a simulated manifest"})
 
 
 def test_create_allocation():
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, status_code=200))
     allocation_uuid = manifester.create_subscription_allocation()
-    breakpoint()
     assert allocation_uuid == "1234567890"

From ac11f42b0aaf0031caa10d66823b9dc281e0c46a Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 15 Nov 2023 15:07:23 -0500
Subject: [PATCH 04/26] Add test for get_manifest(), add docstrings

This commit adds a test for manifester's get_manifest() method, which is
used to generate a manifest and write it locally. It also adds
docstrings to functions in helpers.py and test_manifester.py that did
not have docstrings previously.
---
 manifester/helpers.py    |  5 +++--
 manifester/manifester.py |  4 ++--
 tests/test_manifester.py | 44 ++++++++++++++++++++++++++--------------
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index a70abb2..984179a 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -7,6 +7,7 @@
 
 def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_timeout=1):
     """Re(Try) a function given its args and kwargs up until a max timeout"""
+
     cmd_args = cmd_args if cmd_args else []
     cmd_kwargs = cmd_kwargs if cmd_kwargs else {}
     # If additional debug information is needed, the following log entry can be modified to
@@ -44,8 +45,8 @@ def process_sat_version(sat_version, valid_sat_versions):
     return sat_version
 
 def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
-    # randomish = random.random()
-    # print(randomish, fail_rate/100)
+    """Return an HTTP response code randomly selected from sets of good and bad codes"""
+
     if random.random() > (fail_rate / 100):
         return random.choice(good_codes)
     else:
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 738dfb2..ea60a82 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -132,7 +132,7 @@ def delete_subscription_allocation(self):
             "params": {"force": "true"},
         }
         response = simple_retry(
-            requests.delete,
+            self.requester.delete,
             cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}"],
             cmd_kwargs=data,
         )
@@ -260,7 +260,7 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
                     pool_id=match["id"],
                     entitlement_quantity=subscription_data["quantity"],
                 )
-                # if the above is using simple_rety, it will raise an exception
+                # if the above is using simple_retry, it will raise an exception
                 # and never trigger the following block
                 if add_entitlements.status_code in [404, 429, 500, 504]:
                     verify_entitlements = self.verify_allocation_entitlements(
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index bb910e7..9f5dc17 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -7,50 +7,64 @@
 import pytest
 import random
 
-def test_empty_init(manifest_category="golden_ticket"):
+def test_basic_init(manifest_category="golden_ticket"):
+    """Test that manifester can initialize with the minimum required arguments and verify that resulting object has an access token attribute"""
+
     manifester_inst = Manifester(manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None))
-    breakpoint()
     assert isinstance(manifester_inst, Manifester)
-    assert manifester_inst.access_token == 'this is a simulated access token'
+    assert manifester_inst.access_token == "this is a simulated access token"
 
 class RhsmApiStub(MockStub):
+
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
         self._fail_rate = kwargs.get("fail_rate", 10)
-        self.status_code = fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
+        self.status_code = kwargs.get("status_code") or fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
         super().__init__(in_dict)
 
-    # @property
-    # def status_code(self):
-    #     return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
-
     def post(*args, **kwargs):
+        """Simulate responses to POST requests for RHSM API endpoints used by Manifester"""
+
         if args[1].endswith("openid-connect/token"):
             return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
         if args[1].endswith("allocations/"):
             return RhsmApiStub(in_dict={"uuid": "1234567890"})
         if args[1].endswith("entitlements"):
-            return RhsmApiStub(status_code=200)
+            return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
 
     def get(*args, **kwargs):
+        """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
+
         if args[1].endswith("versions"):
             return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
         if args[1].endswith("pools"):
             # question: how to fake > 50 pools to test use of offset parameter?
-            return MockStub(in_dict={"pool": "this is a simulated list of dictionaries of subscription pool data"})
+            return RhsmApiStub(in_dict={'body': [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]})
         if "allocations" in args[1] and not ("export" in args[1] or "pools" in args[1]):
-            return MockStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
+            return RhsmApiStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
         if args[1].endswith("export"):
-            return MockStub(in_dict={"export_job": "Manifest export job triggered successfully"})
+            return RhsmApiStub(in_dict={'body': {'exportJobID': '123456', 'href': 'exportJob'}})
         if "exportJob" in args[1]:
             responses = [202, 200]
-            return MockStub(status_code=random.choice(responses))
+            return RhsmApiStub(in_dict={'body': {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}}, status_code=random.choice(responses))
         if "export" in args[1] and not args[1].endswith("export"):
-            return MockStub(in_dict={"content": "this is a simulated manifest"})
+            return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
 
 
 def test_create_allocation():
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, status_code=200))
+    """Test that manifester's create_subscription_allocation method returns a UUID"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     allocation_uuid = manifester.create_subscription_allocation()
     assert allocation_uuid == "1234567890"
+
+def test_negative_export_limit_exceeded():
+    """Test that exceeding the limit when checking an export job's status results in an exception"""
+
+def test_get_manifest():
+    """Test that manifester's get_manifest method returns a manifest"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifest = manifester.get_manifest()
+    assert manifest["content"].decode("utf-8") == "this is a simulated manifest"
\ No newline at end of file

From c4880fa51f2ede7548728bda3e130f7a4a1e665f Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Thu, 16 Nov 2023 13:59:31 -0500
Subject: [PATCH 05/26] Add test for delete_subscription_allocation

This commit adds a test for Manifester's
`delete_subscription_allocation` method and a `delete` method to the
`RhsmApiStub` class to handle HTTP DELETE requests.
---
 tests/test_manifester.py | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 9f5dc17..dcda1ec 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -28,7 +28,7 @@ def post(*args, **kwargs):
 
         if args[1].endswith("openid-connect/token"):
             return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
-        if args[1].endswith("allocations/"):
+        if args[1].endswith("allocations"):
             return RhsmApiStub(in_dict={"uuid": "1234567890"})
         if args[1].endswith("entitlements"):
             return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
@@ -51,6 +51,12 @@ def get(*args, **kwargs):
         if "export" in args[1] and not args[1].endswith("export"):
             return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
 
+    def delete(*args, **kwargs):
+        """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester"""
+
+        if args[1].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
+            return RhsmApiStub(in_dict={"content": b""}, good_codes=[204])
+
 
 def test_create_allocation():
     """Test that manifester's create_subscription_allocation method returns a UUID"""
@@ -67,4 +73,14 @@ def test_get_manifest():
 
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifest = manifester.get_manifest()
-    assert manifest["content"].decode("utf-8") == "this is a simulated manifest"
\ No newline at end of file
+    assert manifest.content.decode("utf-8") == "this is a simulated manifest"
+    assert manifest.status_code == 200
+
+def test_delete_subscription_allocation():
+    """Test that manifester's delete_subscription_allocation method deletes a subscription allocation"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester.get_manifest()
+    response = manifester.delete_subscription_allocation()
+    assert response.status_code == 204
+    assert response.content == b""

From d7c486194239da460c10e36ac161963292aa62cb Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Fri, 17 Nov 2023 15:18:50 -0500
Subject: [PATCH 06/26] Add negative test for simple retry's timeout

---
 tests/test_manifester.py | 42 +++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index dcda1ec..bddc074 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -20,41 +20,46 @@ def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
         self._fail_rate = kwargs.get("fail_rate", 10)
-        self.status_code = kwargs.get("status_code") or fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
+        # self.status_code = kwargs.get("status_code") or fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
         super().__init__(in_dict)
 
-    def post(*args, **kwargs):
+    @property
+    def status_code(self):
+        return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
+
+    def post(self, *args, **kwargs):
         """Simulate responses to POST requests for RHSM API endpoints used by Manifester"""
 
-        if args[1].endswith("openid-connect/token"):
+        if args[0].endswith("openid-connect/token"):
             return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
-        if args[1].endswith("allocations"):
+        if args[0].endswith("allocations"):
             return RhsmApiStub(in_dict={"uuid": "1234567890"})
-        if args[1].endswith("entitlements"):
+        if args[0].endswith("entitlements"):
             return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
 
-    def get(*args, **kwargs):
+    def get(self, *args, **kwargs):
         """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
 
-        if args[1].endswith("versions"):
+        if args[0].endswith("versions"):
             return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
-        if args[1].endswith("pools"):
+        if args[0].endswith("pools"):
             # question: how to fake > 50 pools to test use of offset parameter?
             return RhsmApiStub(in_dict={'body': [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]})
-        if "allocations" in args[1] and not ("export" in args[1] or "pools" in args[1]):
+        if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
             return RhsmApiStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
-        if args[1].endswith("export"):
+        if args[0].endswith("export"):
             return RhsmApiStub(in_dict={'body': {'exportJobID': '123456', 'href': 'exportJob'}})
-        if "exportJob" in args[1]:
+        if "exportJob" in args[0]:
             responses = [202, 200]
             return RhsmApiStub(in_dict={'body': {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}}, status_code=random.choice(responses))
-        if "export" in args[1] and not args[1].endswith("export"):
+        if "export" in args[0] and not args[0].endswith("export"):
+            # Manifester expets a bytes-type object to be returned as the manifest
             return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
 
-    def delete(*args, **kwargs):
+    def delete(self, *args, **kwargs):
         """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester"""
 
-        if args[1].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
+        if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
             return RhsmApiStub(in_dict={"content": b""}, good_codes=[204])
 
 
@@ -65,8 +70,13 @@ def test_create_allocation():
     allocation_uuid = manifester.create_subscription_allocation()
     assert allocation_uuid == "1234567890"
 
-def test_negative_export_limit_exceeded():
-    """Test that exceeding the limit when checking an export job's status results in an exception"""
+def test_negative_simple_retry_timeout():
+    """Test that exceeding the attempt limit when retrying a failed API call results in an exception"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=100))
+    with pytest.raises(Exception) as exception:
+        manifester.get_manifest()
+    assert str(exception.value) == "Timeout exceeded"
 
 def test_get_manifest():
     """Test that manifester's get_manifest method returns a manifest"""

From 6f34d36d2f2d40b6dae2ade4c3687453eb225696 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 13:41:49 -0500
Subject: [PATCH 07/26] Add test, modify mock method returns

This commit adds a negative test for the timeout on exporting a
manifest. It also modifies the return values of the mocked HTTP verb
methods to reuse the same RhsmApiStub object throughout each execution,
rather than returning a new RhsmApiStub object with each method call. It
also modifies the manifester class and helper methods to support this
change.
---
 manifester/helpers.py    |  4 +-
 manifester/manifester.py | 26 +++++++++----
 tests/test_manifester.py | 80 ++++++++++++++++++++++++++++++++--------
 3 files changed, 86 insertions(+), 24 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index 984179a..7437cc8 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -5,7 +5,7 @@
 from logzero import logger
 
 
-def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_timeout=1):
+def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=2, _cur_timeout=1):
     """Re(Try) a function given its args and kwargs up until a max timeout"""
 
     cmd_args = cmd_args if cmd_args else []
@@ -19,7 +19,7 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
     if response.status_code in [429, 500, 504]:
         new_wait = _cur_timeout * 2
         if new_wait > max_timeout:
-            raise Exception("Timeout exceeded")
+            raise Exception("Retry timeout exceeded")
         logger.debug(f"Trying again in {_cur_timeout} seconds")
         time.sleep(_cur_timeout)
         response = simple_retry(cmd, cmd_args, cmd_kwargs, max_timeout, new_wait)
diff --git a/manifester/manifester.py b/manifester/manifester.py
index ea60a82..05e72a1 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -58,7 +58,10 @@ def access_token(self):
                 cmd_args=[f"{self.token_request_url}"],
                 cmd_kwargs=token_request_data,
             ).json()
-            self._access_token = token_data["access_token"]
+            if self.is_mock:
+                self._access_token = token_data.access_token
+            else:
+                self._access_token = token_data["access_token"]
         return self._access_token
     
     @cached_property
@@ -131,6 +134,8 @@ def delete_subscription_allocation(self):
             "proxies": self.manifest_data.get("proxies", settings.proxies),
             "params": {"force": "true"},
         }
+        if self.is_mock:
+            self.allocation_uuid = self.allocation_uuid.uuid
         response = simple_retry(
             self.requester.delete,
             cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}"],
@@ -241,11 +246,14 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
 
     def process_subscription_pools(self, subscription_pools, subscription_data):
         logger.debug(f"Finding a matching pool for {subscription_data['name']}.")
-        matching = [
-            d
-            for d in subscription_pools["body"]
-            if d["subscriptionName"] == subscription_data["name"]
-        ]
+        if self.is_mock:
+            matching = [d for d in subscription_pools.body if d["subscriptionName"] == subscription_data["name"]]
+        else:
+            matching = [
+                d
+                for d in subscription_pools["body"]
+                if d["subscriptionName"] == subscription_data["name"]
+            ]
         logger.debug(
             f"The following pools are matches for this subscription: {matching}"
         )
@@ -355,13 +363,17 @@ def trigger_manifest_export(self):
                     "Manifest export job status check limit exceeded. This may indicate an "
                     "upstream issue with Red Hat Subscription Management."
                 )
+                raise Exception("Export timeout exceeded")
                 break
             request_count += 1
         if limit_exceeded:
             self.content = None
             return self
         export_job = export_job.json()
-        export_href = export_job["body"]["href"]
+        if self.is_mock:
+            export_href = export_job.body["href"]
+        else:
+            export_href = export_job["body"]["href"]
         manifest = simple_retry(
             self.requester.get,
             cmd_args=[f"{export_href}"],
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index bddc074..22c4c26 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -1,5 +1,6 @@
 from unittest.mock import Mock
 
+from functools import cached_property
 from requests import request
 from manifester import Manifester
 from manifester.settings import settings
@@ -23,7 +24,7 @@ def __init__(self, in_dict=None, **kwargs):
         # self.status_code = kwargs.get("status_code") or fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
         super().__init__(in_dict)
 
-    @property
+    @cached_property
     def status_code(self):
         return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
 
@@ -31,36 +32,76 @@ def post(self, *args, **kwargs):
         """Simulate responses to POST requests for RHSM API endpoints used by Manifester"""
 
         if args[0].endswith("openid-connect/token"):
-            return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
+            self.access_token = "this is a simulated access token"
+            return self
         if args[0].endswith("allocations"):
-            return RhsmApiStub(in_dict={"uuid": "1234567890"})
+            self.uuid = "1234567890"
+            return self
         if args[0].endswith("entitlements"):
-            return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
+            self.params = kwargs["params"]
+            return self
+        # if args[0].endswith("openid-connect/token"):
+        #     return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
+        # if args[0].endswith("allocations"):
+        #     return RhsmApiStub(in_dict={"uuid": "1234567890"})
+        # if args[0].endswith("entitlements"):
+        #     return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
 
     def get(self, *args, **kwargs):
         """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
 
         if args[0].endswith("versions"):
-            return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
+            self.valid_sat_versions = ["sat-6.12", "sat-6.13", "sat-6.14"]
+            return self
         if args[0].endswith("pools"):
-            # question: how to fake > 50 pools to test use of offset parameter?
-            return RhsmApiStub(in_dict={'body': [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]})
+            self.body = [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]
+            return self
         if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
-            return RhsmApiStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
+            self.allocation_data = "this allocation data also includes entitlement data"
+            return self
         if args[0].endswith("export"):
-            return RhsmApiStub(in_dict={'body': {'exportJobID': '123456', 'href': 'exportJob'}})
+            self.body = {'exportJobID': '123456', 'href': 'exportJob'}
+            return self
         if "exportJob" in args[0]:
-            responses = [202, 200]
-            return RhsmApiStub(in_dict={'body': {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}}, status_code=random.choice(responses))
+            del self.status_code
+            if self.force_export_failure:
+                self._good_codes = [202]
+            else:
+                self._good_codes = [202, 200]
+            self.body = {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}
+            return self
         if "export" in args[0] and not args[0].endswith("export"):
+            del self.status_code
+            self._good_codes = [200]
             # Manifester expets a bytes-type object to be returned as the manifest
-            return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
+            self.content = b"this is a simulated manifest"
+            return self
+        # if args[0].endswith("versions"):
+        #     return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
+        # if args[0].endswith("pools"):
+        #     # question: how to fake > 50 pools to test use of offset parameter?
+        #     return RhsmApiStub(in_dict={'body': [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]})
+        # if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
+        #     return RhsmApiStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
+        # if args[0].endswith("export"):
+        #     return RhsmApiStub(in_dict={'body': {'exportJobID': '123456', 'href': 'exportJob'}})
+        # if "exportJob" in args[0]:
+        #     responses = [202, 200]
+        #     return RhsmApiStub(in_dict={'body': {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}}, status_code=random.choice(responses))
+        # if "export" in args[0] and not args[0].endswith("export"):
+        #     # Manifester expets a bytes-type object to be returned as the manifest
+        #     return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
 
     def delete(self, *args, **kwargs):
         """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester"""
 
         if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
-            return RhsmApiStub(in_dict={"content": b""}, good_codes=[204])
+            del self.status_code
+            self.content = b""
+            self._good_codes=[204]
+            return self
+        # if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
+        #     return RhsmApiStub(in_dict={"content": b""}, good_codes=[204])
 
 
 def test_create_allocation():
@@ -68,15 +109,24 @@ def test_create_allocation():
 
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     allocation_uuid = manifester.create_subscription_allocation()
-    assert allocation_uuid == "1234567890"
+    assert allocation_uuid.uuid == "1234567890"
 
 def test_negative_simple_retry_timeout():
     """Test that exceeding the attempt limit when retrying a failed API call results in an exception"""
 
+    # TODO: figure out why this test fails despite raising the expected exception
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=100))
+    with pytest.raises(Exception) as exception:
+        manifester.create_subscription_allocation()
+    assert str(exception.value) == "Retry timeout exceeded"
+
+def test_negative_manifest_export_timeout():
+    """Test that exceeding the attempt limit when exporting a manifest results in an exception"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict={"force_export_failure": True}))
     with pytest.raises(Exception) as exception:
         manifester.get_manifest()
-    assert str(exception.value) == "Timeout exceeded"
+    assert str(exception.value) == "Export timeout exceeded"
 
 def test_get_manifest():
     """Test that manifester's get_manifest method returns a manifest"""

From 809bd13919df85940caec5dc153f49622fb42ac2 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 13:49:28 -0500
Subject: [PATCH 08/26] Remove extraneous comments

---
 tests/test_manifester.py | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 22c4c26..28b354a 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -21,7 +21,6 @@ def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
         self._fail_rate = kwargs.get("fail_rate", 10)
-        # self.status_code = kwargs.get("status_code") or fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
         super().__init__(in_dict)
 
     @cached_property
@@ -40,12 +39,6 @@ def post(self, *args, **kwargs):
         if args[0].endswith("entitlements"):
             self.params = kwargs["params"]
             return self
-        # if args[0].endswith("openid-connect/token"):
-        #     return RhsmApiStub(in_dict={"access_token": "this is a simulated access token"})
-        # if args[0].endswith("allocations"):
-        #     return RhsmApiStub(in_dict={"uuid": "1234567890"})
-        # if args[0].endswith("entitlements"):
-        #     return RhsmApiStub(in_dict={"params": kwargs["params"]}, status_code=200)
 
     def get(self, *args, **kwargs):
         """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
@@ -73,24 +66,9 @@ def get(self, *args, **kwargs):
         if "export" in args[0] and not args[0].endswith("export"):
             del self.status_code
             self._good_codes = [200]
-            # Manifester expets a bytes-type object to be returned as the manifest
+            # Manifester expects a bytes-type object to be returned as the manifest
             self.content = b"this is a simulated manifest"
             return self
-        # if args[0].endswith("versions"):
-        #     return RhsmApiStub(in_dict={"valid_sat_versions": ["sat-6.12", "sat-6.13", "sat-6.14"]})
-        # if args[0].endswith("pools"):
-        #     # question: how to fake > 50 pools to test use of offset parameter?
-        #     return RhsmApiStub(in_dict={'body': [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]})
-        # if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
-        #     return RhsmApiStub(in_dict={"allocation_data": "this allocation data also includes entitlement data"})
-        # if args[0].endswith("export"):
-        #     return RhsmApiStub(in_dict={'body': {'exportJobID': '123456', 'href': 'exportJob'}})
-        # if "exportJob" in args[0]:
-        #     responses = [202, 200]
-        #     return RhsmApiStub(in_dict={'body': {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}}, status_code=random.choice(responses))
-        # if "export" in args[0] and not args[0].endswith("export"):
-        #     # Manifester expets a bytes-type object to be returned as the manifest
-        #     return RhsmApiStub(in_dict={"content": b"this is a simulated manifest"})
 
     def delete(self, *args, **kwargs):
         """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester"""
@@ -100,8 +78,6 @@ def delete(self, *args, **kwargs):
             self.content = b""
             self._good_codes=[204]
             return self
-        # if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
-        #     return RhsmApiStub(in_dict={"content": b""}, good_codes=[204])
 
 
 def test_create_allocation():

From 3202681f1cf97c2c7613f5d009db6b7f96db4f28 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 14:21:10 -0500
Subject: [PATCH 09/26] Revert max_timeout change

---
 manifester/helpers.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index 7437cc8..3b1bd01 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -5,7 +5,7 @@
 from logzero import logger
 
 
-def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=2, _cur_timeout=1):
+def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_timeout=1):
     """Re(Try) a function given its args and kwargs up until a max timeout"""
 
     cmd_args = cmd_args if cmd_args else []

From 1149cb790bb2158db5137981c3a87059d71fa045 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 14:27:33 -0500
Subject: [PATCH 10/26] Correct logic error in sat version evaluation

---
 manifester/manifester.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/manifester/manifester.py b/manifester/manifester.py
index 05e72a1..3965a99 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -78,11 +78,10 @@ def valid_sat_versions(self):
                 ],
             cmd_kwargs=headers,
             ).json()
-        if self.is_mock is False:
-            for ver_dict in sat_versions_response["body"]:
-                valid_sat_versions.append(ver_dict["value"])
-        else:
-            valid_sat_versions = sat_versions_response["valid_sat_versions"]
+        if self.is_mock:
+            valid_sat_versions = sat_versions_response.valid_sat_versions
+        for ver_dict in sat_versions_response["body"]:
+            valid_sat_versions.append(ver_dict["value"])
         return valid_sat_versions
 
     def create_subscription_allocation(self):

From f992b472dcc5adae1c0610e75761b1503f6cad9e Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 14:44:25 -0500
Subject: [PATCH 11/26] Use more realistic data in version check

---
 manifester/manifester.py | 3 ++-
 tests/test_manifester.py | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/manifester/manifester.py b/manifester/manifester.py
index 3965a99..a795c91 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -78,8 +78,9 @@ def valid_sat_versions(self):
                 ],
             cmd_kwargs=headers,
             ).json()
+        breakpoint()
         if self.is_mock:
-            valid_sat_versions = sat_versions_response.valid_sat_versions
+            sat_versions_response = sat_versions_response.version_response
         for ver_dict in sat_versions_response["body"]:
             valid_sat_versions.append(ver_dict["value"])
         return valid_sat_versions
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 28b354a..495c8b4 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -44,7 +44,11 @@ def get(self, *args, **kwargs):
         """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
 
         if args[0].endswith("versions"):
-            self.valid_sat_versions = ["sat-6.12", "sat-6.13", "sat-6.14"]
+            self.version_response = {'body': [
+                {'value': 'sat-6.14', 'description': 'Satellite 6.14'},
+                {'value': 'sat-6.13', 'description': 'Satellite 6.13'},
+                {'value': 'sat-6.12', 'description': 'Satellite 6.12'}
+            ]}
             return self
         if args[0].endswith("pools"):
             self.body = [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]

From 6a43c1ca4c122f31aa9011ae842cceffb4c8e387 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 21 Nov 2023 16:15:59 -0500
Subject: [PATCH 12/26] Fix failing test for simple retry timeout

---
 manifester/helpers.py    |  1 +
 manifester/manifester.py |  1 -
 tests/test_manifester.py | 12 +++++++-----
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index 3b1bd01..398fa3c 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -19,6 +19,7 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
     if response.status_code in [429, 500, 504]:
         new_wait = _cur_timeout * 2
         if new_wait > max_timeout:
+            breakpoint()
             raise Exception("Retry timeout exceeded")
         logger.debug(f"Trying again in {_cur_timeout} seconds")
         time.sleep(_cur_timeout)
diff --git a/manifester/manifester.py b/manifester/manifester.py
index a795c91..98c5255 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -78,7 +78,6 @@ def valid_sat_versions(self):
                 ],
             cmd_kwargs=headers,
             ).json()
-        breakpoint()
         if self.is_mock:
             sat_versions_response = sat_versions_response.version_response
         for ver_dict in sat_versions_response["body"]:
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 495c8b4..5b2e318 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -94,18 +94,17 @@ def test_create_allocation():
 def test_negative_simple_retry_timeout():
     """Test that exceeding the attempt limit when retrying a failed API call results in an exception"""
 
-    # TODO: figure out why this test fails despite raising the expected exception
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=100))
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=0))
+    manifester.requester._fail_rate = 100
     with pytest.raises(Exception) as exception:
-        manifester.create_subscription_allocation()
+        manifester.get_manifest()
     assert str(exception.value) == "Retry timeout exceeded"
 
 def test_negative_manifest_export_timeout():
     """Test that exceeding the attempt limit when exporting a manifest results in an exception"""
 
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict={"force_export_failure": True}))
     with pytest.raises(Exception) as exception:
-        manifester.get_manifest()
+        manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict={"force_export_failure": True}))
     assert str(exception.value) == "Export timeout exceeded"
 
 def test_get_manifest():
@@ -124,3 +123,6 @@ def test_delete_subscription_allocation():
     response = manifester.delete_subscription_allocation()
     assert response.status_code == 204
     assert response.content == b""
+
+def test_ingest_manifest_data_via_dict():
+    """Test that manifester is able to """

From f2a897eeb9baab782991eba81ccfa60756716f8b Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 22 Nov 2023 11:45:52 -0500
Subject: [PATCH 13/26] Add test for reading manifest data from dictionary

---
 tests/test_manifester.py | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 5b2e318..bf3e262 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -125,4 +125,33 @@ def test_delete_subscription_allocation():
     assert response.content == b""
 
 def test_ingest_manifest_data_via_dict():
-    """Test that manifester is able to """
+    """Test that manifester is able to read configuration data from a dictionary"""
+
+    manifest_data = {
+        "log_level": "debug",
+        "offline_token": "test",
+        "proxies": {"https:" ""},
+        "url": {
+            "token_request": "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
+            "allocations": "https://api.access.redhat.com/management/v1/allocations",
+        },
+        "sat_version": "sat-6.14",
+        "subscription_data": [
+            {
+                "name": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)", 
+                "quantity": 1,
+                },
+            {
+                "name": "Red Hat Satellite Infrastructure Subscription",
+                "quantity": 1
+            },
+        ],
+        "simple_content_access": "enabled",
+    }
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
+    assert manifester.subscription_data == manifest_data["subscription_data"]
+    assert manifester.simple_content_access == manifest_data["simple_content_access"]
+    assert manifester.token_request_url == manifest_data["url"]["token_request"]
+    assert manifester.allocations_url == manifest_data["url"]["allocations"]
+    assert manifester.sat_version == manifest_data["sat_version"]
+

From fb44056b7871ec191132651334de3b1abd48f8e4 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 28 Nov 2023 10:43:03 -0500
Subject: [PATCH 14/26] Add test of subscription pool retrieval with offset

This commit adds a test of manifester's ability to handle the RHSM API's
result pagination when attempting to retrieve all the subscription pools
in an account containing more than 50 subscription pools. It also
modifies the Manifester and RhsmApiStub classes to support the test.
---
 manifester/helpers.py    |  1 -
 manifester/manifester.py | 17 ++++++++++---
 tests/test_manifester.py | 53 +++++++++++++++++++++++++++++++---------
 3 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index 398fa3c..3b1bd01 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -19,7 +19,6 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
     if response.status_code in [429, 500, 504]:
         new_wait = _cur_timeout * 2
         if new_wait > max_timeout:
-            breakpoint()
             raise Exception("Retry timeout exceeded")
         logger.debug(f"Trying again in {_cur_timeout} seconds")
         time.sleep(_cur_timeout)
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 98c5255..9ab6ea6 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -158,7 +158,10 @@ def subscription_pools(self):
                 ],
                 cmd_kwargs=data,
             ).json()
-            _results = len(self._subscription_pools["body"])
+            if self.is_mock:
+                _results = len(self._subscription_pools.body)
+            else:
+                _results = len(self._subscription_pools["body"])
             # The endpoint used in the above API call can return a maximum of 50 subscription pools.
             # For organizations with more than 50 subscription pools, the loop below works around
             # this limit by repeating calls with a progressively larger value for the `offset`
@@ -180,9 +183,14 @@ def subscription_pools(self):
                     ],
                     cmd_kwargs=data,
                 ).json()
-                self._subscription_pools["body"] += offset_pools["body"]
-                _results = len(offset_pools["body"])
-                total_pools = len(self._subscription_pools["body"])
+                if self.is_mock:
+                    self._subscription_pools.body += offset_pools.body
+                    _results = len(offset_pools.body)
+                    total_pools = len(self._subscription_pools.body)
+                else:
+                    self._subscription_pools["body"] += offset_pools["body"]
+                    _results = len(offset_pools["body"])
+                    total_pools = len(self._subscription_pools["body"])
                 logger.debug(
                     f"Total subscription pools available for this allocation: {total_pools}"
                 )
@@ -346,6 +354,7 @@ def trigger_manifest_export(self):
         request_count = 1
         limit_exceeded = False
         while export_job.status_code != 200:
+            print(export_job.status_code)
             export_job = simple_retry(
                 self.requester.get,
                 cmd_args=[
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index bf3e262..aed6d87 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -7,13 +7,7 @@
 from manifester.helpers import MockStub, fake_http_response_code
 import pytest
 import random
-
-def test_basic_init(manifest_category="golden_ticket"):
-    """Test that manifester can initialize with the minimum required arguments and verify that resulting object has an access token attribute"""
-
-    manifester_inst = Manifester(manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None))
-    assert isinstance(manifester_inst, Manifester)
-    assert manifester_inst.access_token == "this is a simulated access token"
+import string
 
 class RhsmApiStub(MockStub):
 
@@ -47,12 +41,34 @@ def get(self, *args, **kwargs):
             self.version_response = {'body': [
                 {'value': 'sat-6.14', 'description': 'Satellite 6.14'},
                 {'value': 'sat-6.13', 'description': 'Satellite 6.13'},
-                {'value': 'sat-6.12', 'description': 'Satellite 6.12'}
+                {'value': 'sat-6.12', 'description': 'Satellite 6.12'},
             ]}
             return self
-        if args[0].endswith("pools"):
-            self.body = [{'id': '987adf2a8977', 'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 'entitlementsAvailable': 13}]
+        if args[0].endswith("pools") and not self.has_offset:
+            self.body = [{
+                'id': '987adf2a8977', 
+                'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
+                'entitlementsAvailable': 13,
+                }]
             return self
+        if args[0].endswith("pools") and self.has_offset:
+            if kwargs["params"]["offset"] != 50:
+                self.body = []
+                for x in range(50):
+                    self.body.append({
+                        'id': f'{"".join(random.sample(string.ascii_letters, 12))}', 
+                        'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 
+                        'entitlementsAvailable': random.randrange(100),
+                        })
+                return self
+            else:
+                self.body += [{
+                    'id': '987adf2a8977', 
+                    'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
+                    'entitlementsAvailable': 13,
+                    }]
+                return self
+
         if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
             self.allocation_data = "this allocation data also includes entitlement data"
             return self
@@ -61,7 +77,7 @@ def get(self, *args, **kwargs):
             return self
         if "exportJob" in args[0]:
             del self.status_code
-            if self.force_export_failure:
+            if self.force_export_failure is True and not self.has_offset:
                 self._good_codes = [202]
             else:
                 self._good_codes = [202, 200]
@@ -84,6 +100,13 @@ def delete(self, *args, **kwargs):
             return self
 
 
+def test_basic_init(manifest_category="golden_ticket"):
+    """Test that manifester can initialize with the minimum required arguments and verify that resulting object has an access token attribute"""
+
+    manifester_inst = Manifester(manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None))
+    assert isinstance(manifester_inst, Manifester)
+    assert manifester_inst.access_token == "this is a simulated access token"
+
 def test_create_allocation():
     """Test that manifester's create_subscription_allocation method returns a UUID"""
 
@@ -155,3 +178,11 @@ def test_ingest_manifest_data_via_dict():
     assert manifester.allocations_url == manifest_data["url"]["allocations"]
     assert manifester.sat_version == manifest_data["sat_version"]
 
+def test_get_subscription_pools_with_offset():
+    """Tests that manifester can retrieve all subscription pools from an account containing more than 
+    50 pools"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester.requester.has_offset = True
+    manifester.get_manifest()
+    assert len(manifester.subscription_pools.body) > 50
\ No newline at end of file

From 7e45b04e8a0212418624fb14b4afabf69fe77bb9 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 28 Nov 2023 14:29:23 -0500
Subject: [PATCH 15/26] Use realistic data to simplify mock handling

This commit brings the mock subscription pool response data more in line
with what is returned by the RHSM API in order to simplify some of the
conditional logic that the Manifester class uses to process real vs
mocked data.
---
 manifester/manifester.py | 32 ++++++++++-----------
 tests/test_manifester.py | 60 +++++++++++++++++++++++++---------------
 2 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/manifester/manifester.py b/manifester/manifester.py
index 9ab6ea6..0deadad 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -43,6 +43,7 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
         self.allocations_url = self.manifest_data.get("url", {}).get("allocations", settings.url.allocations)
         self._access_token = None
         self._subscription_pools = None
+        self._active_pools = []
         self.sat_version = process_sat_version(
             kwargs.get("sat_version", self.manifest_data.sat_version),
             self.valid_sat_versions,
@@ -159,9 +160,8 @@ def subscription_pools(self):
                 cmd_kwargs=data,
             ).json()
             if self.is_mock:
-                _results = len(self._subscription_pools.body)
-            else:
-                _results = len(self._subscription_pools["body"])
+                self._subscription_pools = self._subscription_pools.pool_response
+            _results = len(self._subscription_pools["body"])
             # The endpoint used in the above API call can return a maximum of 50 subscription pools.
             # For organizations with more than 50 subscription pools, the loop below works around
             # this limit by repeating calls with a progressively larger value for the `offset`
@@ -184,13 +184,10 @@ def subscription_pools(self):
                     cmd_kwargs=data,
                 ).json()
                 if self.is_mock:
-                    self._subscription_pools.body += offset_pools.body
-                    _results = len(offset_pools.body)
-                    total_pools = len(self._subscription_pools.body)
-                else:
-                    self._subscription_pools["body"] += offset_pools["body"]
-                    _results = len(offset_pools["body"])
-                    total_pools = len(self._subscription_pools["body"])
+                    offset_pools = offset_pools.pool_response
+                self._subscription_pools["body"] += offset_pools["body"]
+                _results = len(offset_pools["body"])
+                total_pools = len(self._subscription_pools["body"])
                 logger.debug(
                     f"Total subscription pools available for this allocation: {total_pools}"
                 )
@@ -253,14 +250,11 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
 
     def process_subscription_pools(self, subscription_pools, subscription_data):
         logger.debug(f"Finding a matching pool for {subscription_data['name']}.")
-        if self.is_mock:
-            matching = [d for d in subscription_pools.body if d["subscriptionName"] == subscription_data["name"]]
-        else:
-            matching = [
-                d
-                for d in subscription_pools["body"]
-                if d["subscriptionName"] == subscription_data["name"]
-            ]
+        matching = [
+            d
+            for d in subscription_pools["body"]
+            if d["subscriptionName"] == subscription_data["name"]
+        ]
         logger.debug(
             f"The following pools are matches for this subscription: {matching}"
         )
@@ -312,12 +306,14 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
                             f"Successfully added {subscription_data['quantity']} entitlements of "
                             f"{subscription_data['name']} to the allocation."
                         )
+                        self._active_pools.append(match)
                         break
                 elif add_entitlements.status_code == 200:
                     logger.debug(
                         f"Successfully added {subscription_data['quantity']} entitlements of "
                         f"{subscription_data['name']} to the allocation."
                     )
+                    self._active_pools.append(match)
                     break
                 else:
                     raise Exception(
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index aed6d87..5e2c404 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -8,13 +8,40 @@
 import pytest
 import random
 import string
-
+import uuid
+
+sub_pool_response = {
+    'body':
+    [
+        {
+            'id': f'{uuid.uuid4().hex}',
+            'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
+            'entitlementsAvailable': 8,
+        },
+        {
+            'id': f'{uuid.uuid4().hex}',
+            'subscriptionName': 'Red Hat Enterprise Linux for Virtual Datacenters, Premium',
+            'entitlementsAvailable': 8,
+        },
+        {
+            'id': f'{uuid.uuid4().hex}',
+            'subscriptionName': 'Red Hat Beta Access',
+            'entitlementsAvailable': 8,
+        },
+        {
+            'id': f'{uuid.uuid4().hex}',
+            'subscriptionName': 'Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)',
+            'entitlementsAvailable': 8,
+        },
+    ],
+}
 class RhsmApiStub(MockStub):
 
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
         self._fail_rate = kwargs.get("fail_rate", 10)
+        self._has_offset = kwargs.get("has_offset", False)
         super().__init__(in_dict)
 
     @cached_property
@@ -44,31 +71,22 @@ def get(self, *args, **kwargs):
                 {'value': 'sat-6.12', 'description': 'Satellite 6.12'},
             ]}
             return self
-        if args[0].endswith("pools") and not self.has_offset:
-            self.body = [{
-                'id': '987adf2a8977', 
-                'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
-                'entitlementsAvailable': 13,
-                }]
+        if args[0].endswith("pools") and not self._has_offset:
+            self.pool_response = sub_pool_response
             return self
-        if args[0].endswith("pools") and self.has_offset:
+        if args[0].endswith("pools") and self._has_offset:
             if kwargs["params"]["offset"] != 50:
-                self.body = []
+                self.pool_response = {'body': []}
                 for x in range(50):
-                    self.body.append({
+                    self.pool_response["body"].append({
                         'id': f'{"".join(random.sample(string.ascii_letters, 12))}', 
                         'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 
                         'entitlementsAvailable': random.randrange(100),
                         })
                 return self
             else:
-                self.body += [{
-                    'id': '987adf2a8977', 
-                    'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
-                    'entitlementsAvailable': 13,
-                    }]
+                self.pool_response["body"] += sub_pool_response["body"]
                 return self
-
         if "allocations" in args[0] and not ("export" in args[0] or "pools" in args[0]):
             self.allocation_data = "this allocation data also includes entitlement data"
             return self
@@ -77,7 +95,7 @@ def get(self, *args, **kwargs):
             return self
         if "exportJob" in args[0]:
             del self.status_code
-            if self.force_export_failure is True and not self.has_offset:
+            if self.force_export_failure is True and not self._has_offset:
                 self._good_codes = [202]
             else:
                 self._good_codes = [202, 200]
@@ -179,10 +197,8 @@ def test_ingest_manifest_data_via_dict():
     assert manifester.sat_version == manifest_data["sat_version"]
 
 def test_get_subscription_pools_with_offset():
-    """Tests that manifester can retrieve all subscription pools from an account containing more than 
-    50 pools"""
+    """Tests that manifester can retrieve all subscription pools from an account containing more than 50 pools"""
 
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
-    manifester.requester.has_offset = True
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True))
     manifester.get_manifest()
-    assert len(manifester.subscription_pools.body) > 50
\ No newline at end of file
+    assert len(manifester.subscription_pools["body"]) > 50

From 4fe55e08e506780c6bd4076375849d9ec1a35234 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 28 Nov 2023 14:45:30 -0500
Subject: [PATCH 16/26] Add test for correct subs in allocation

This commit adds a test to ensure that each of the subscriptions in
manifester_settings.yaml was successfully added to the subscription
allocation created during a `get_manifest()` execution.
---
 tests/test_manifester.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 5e2c404..4f03fc0 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -202,3 +202,12 @@ def test_get_subscription_pools_with_offset():
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True))
     manifester.get_manifest()
     assert len(manifester.subscription_pools["body"]) > 50
+
+def test_correct_subs_added_to_allocation():
+    """Test that the subscriptions added to the subscription allocation match the subscription data defined in manifester's config"""
+
+    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester.get_manifest()
+    sub_names_from_config = [ x["NAME"] for x in manifester.subscription_data ]
+    for pool in manifester._active_pools:
+        assert pool["subscriptionName"] in sub_names_from_config

From 341cada11e7ccd96e8dc1a957db262d37b87307f Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 28 Nov 2023 15:36:06 -0500
Subject: [PATCH 17/26] Tweak test_correct_subs_added_to_allocation

---
 tests/test_manifester.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 4f03fc0..8e0b888 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -208,6 +208,6 @@ def test_correct_subs_added_to_allocation():
 
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
-    sub_names_from_config = [ x["NAME"] for x in manifester.subscription_data ]
-    for pool in manifester._active_pools:
-        assert pool["subscriptionName"] in sub_names_from_config
+    active_subs = sorted([ x["subscriptionName"] for x in manifester._active_pools ])
+    sub_names_from_config = sorted([ x["NAME"] for x in manifester.subscription_data ])
+    assert active_subs == sub_names_from_config

From 83156c27f0a7582f62ce13e1e039033062f3970e Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 29 Nov 2023 15:43:18 -0500
Subject: [PATCH 18/26] Add test for invalid sat_version

This commit adds a test to ensure that Manifester will replace an
invalid sat_version value with the most recent valid sat_version. Since
this test reuses the same manifest_data that was previously defined in
another test, it also moves that manifest_data to a global variable.
---
 tests/test_manifester.py | 50 +++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 8e0b888..208064e 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -10,6 +10,28 @@
 import string
 import uuid
 
+manifest_data = {
+    "log_level": "debug",
+    "offline_token": "test",
+    "proxies": {"https:" ""},
+    "url": {
+        "token_request": "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
+        "allocations": "https://api.access.redhat.com/management/v1/allocations",
+    },
+    "sat_version": "sat-6.14",
+    "subscription_data": [
+        {
+            "name": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)", 
+            "quantity": 1,
+            },
+        {
+            "name": "Red Hat Satellite Infrastructure Subscription",
+            "quantity": 1
+        },
+    ],
+    "simple_content_access": "enabled",
+}
+
 sub_pool_response = {
     'body':
     [
@@ -168,27 +190,6 @@ def test_delete_subscription_allocation():
 def test_ingest_manifest_data_via_dict():
     """Test that manifester is able to read configuration data from a dictionary"""
 
-    manifest_data = {
-        "log_level": "debug",
-        "offline_token": "test",
-        "proxies": {"https:" ""},
-        "url": {
-            "token_request": "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
-            "allocations": "https://api.access.redhat.com/management/v1/allocations",
-        },
-        "sat_version": "sat-6.14",
-        "subscription_data": [
-            {
-                "name": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)", 
-                "quantity": 1,
-                },
-            {
-                "name": "Red Hat Satellite Infrastructure Subscription",
-                "quantity": 1
-            },
-        ],
-        "simple_content_access": "enabled",
-    }
     manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     assert manifester.subscription_data == manifest_data["subscription_data"]
     assert manifester.simple_content_access == manifest_data["simple_content_access"]
@@ -211,3 +212,10 @@ def test_correct_subs_added_to_allocation():
     active_subs = sorted([ x["subscriptionName"] for x in manifester._active_pools ])
     sub_names_from_config = sorted([ x["NAME"] for x in manifester.subscription_data ])
     assert active_subs == sub_names_from_config
+
+def test_invalid_sat_version():
+    """Test that an invalid sat_version value will be replaced with the latest valid sat_version"""
+
+    manifest_data["sat_version"] = "sat-6.20"
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
+    assert manifester.sat_version == "sat-6.14"
\ No newline at end of file

From f53bfaa8c1f55146a98381aa6e0820f8dccacdf8 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Wed, 29 Nov 2023 17:02:46 -0500
Subject: [PATCH 19/26] Add pre-commit hooks and run pre-commit

This commit adds reworks the pre-commit hooks used by manifester. It
includes changes to several files due to reformatting and other changes
from black and ruff.
---
 .pre-commit-config.yaml  | 43 ++++++++----------
 manifester/commands.py   |  4 +-
 manifester/helpers.py    | 10 +++--
 manifester/manifester.py | 95 ++++++++++++++++------------------------
 manifester/settings.py   |  3 +-
 pyproject.toml           | 62 ++++++++++++++++++++++++++
 setup.py                 |  1 -
 tests/test_manifester.py | 95 +++++++++++++++++++++++++---------------
 8 files changed, 188 insertions(+), 125 deletions(-)
 create mode 100644 pyproject.toml

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index c482f05..731998e 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,27 +1,22 @@
 # configuration for pre-commit git hooks
 
 repos:
-- repo: https://github.com/asottile/reorder_python_imports
-  rev: v3.0.1
-  hooks:
-  - id: reorder-python-imports
-- repo: https://github.com/asottile/pyupgrade
-  rev: v2.32.0
-  hooks:
-  - id: pyupgrade
-    args: [--py36-plus]
-- repo: https://github.com/psf/black
-  rev: 22.3.0
-  hooks:
-  - id: black
-- repo: https://gitlab.com/pycqa/flake8
-  rev: 3.9.2
-  hooks:
-  - id: flake8
-- repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.2.0
-  hooks:
-  - id: trailing-whitespace
-  - id: end-of-file-fixer
-  - id: check-yaml
-  - id: debug-statements
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.4.0
+    hooks:
+    - id: trailing-whitespace
+    - id: check-yaml
+    - id: debug-statements
+  - repo: https://github.com/psf/black
+    rev: 22.10.0
+    hooks:
+    - id: black
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.0.277
+    hooks:
+      - id: ruff
+        args: [--fix, --exit-non-zero-on-fix]
+  - repo: https://github.com/gitleaks/gitleaks
+    rev: v8.18.0
+    hooks:
+      - id: gitleaks
\ No newline at end of file
diff --git a/manifester/commands.py b/manifester/commands.py
index 258c561..c5e6b98 100644
--- a/manifester/commands.py
+++ b/manifester/commands.py
@@ -15,9 +15,7 @@ def cli():
     type=str,
     help="Category of manifest (golden_ticket or robottelo_automation by default)",
 )
-@click.option(
-    "--allocation_name", type=str, help="Name of upstream subscription allocation"
-)
+@click.option("--allocation_name", type=str, help="Name of upstream subscription allocation")
 def get_manifest(manifest_category, allocation_name):
     manifester = Manifester(manifest_category, allocation_name)
     manifester.create_subscription_allocation()
diff --git a/manifester/helpers.py b/manifester/helpers.py
index 3b1bd01..22473b2 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -1,7 +1,7 @@
+from collections import UserDict
 import random
 import time
 
-from collections import UserDict
 from logzero import logger
 
 
@@ -25,9 +25,10 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
         response = simple_retry(cmd, cmd_args, cmd_kwargs, max_timeout, new_wait)
     return response
 
+
 def process_sat_version(sat_version, valid_sat_versions):
     """Ensure that the sat_version parameter is properly formatted for the RHSM API when creating
-       a subscription allocation with the 'POST allocations' endpoint"""
+    a subscription allocation with the 'POST allocations' endpoint"""
     if sat_version not in valid_sat_versions:
         # The valid values for the sat_version parameter when creating a subscription allocation
         # are all 8 characters or less (e.g. 'sat-6.11'). Some data sources may include a Z-stream
@@ -40,10 +41,13 @@ def process_sat_version(sat_version, valid_sat_versions):
             sat_version = ".".join(sat_version)
         # If sat_version is still not valid, default to the latest valid version.
         if sat_version not in valid_sat_versions:
-            valid_sat_versions.sort(key = lambda i: int(i.split('-')[-1].split('.')[-1]), reverse = True)
+            valid_sat_versions.sort(
+                key=lambda i: int(i.split('-')[-1].split('.')[-1]), reverse=True
+            )
             return valid_sat_versions[0]
     return sat_version
 
+
 def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
     """Return an HTTP response code randomly selected from sets of good and bad codes"""
 
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 0deadad..4d55518 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -1,14 +1,12 @@
-import random
-import string
-from dynaconf.utils.boxing import DynaBox
 from functools import cached_property
 from pathlib import Path
+import random
+import string
 
+from dynaconf.utils.boxing import DynaBox
 from logzero import logger
 
-from manifester.helpers import simple_retry
-from manifester.helpers import process_sat_version
-from manifester.logger import setup_logzero
+from manifester.helpers import process_sat_version, simple_retry
 from manifester.settings import settings
 
 
@@ -23,11 +21,10 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
             self.is_mock = True
         else:
             import requests
+
             self.requester = requests
             self.is_mock = False
-        self.allocation_name = allocation_name or "".join(
-            random.sample(string.ascii_letters, 10)
-        )
+        self.allocation_name = allocation_name or "".join(random.sample(string.ascii_letters, 10))
         self.manifest_name = Path(f'{self.allocation_name}_manifest.zip')
         self.offline_token = kwargs.get("offline_token", self.manifest_data.offline_token)
         self.subscription_data = self.manifest_data.subscription_data
@@ -39,8 +36,12 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
         self.simple_content_access = kwargs.get(
             "simple_content_access", self.manifest_data.simple_content_access
         )
-        self.token_request_url = self.manifest_data.get("url", {}).get("token_request", settings.url.token_request)
-        self.allocations_url = self.manifest_data.get("url", {}).get("allocations", settings.url.allocations)
+        self.token_request_url = self.manifest_data.get("url", {}).get(
+            "token_request", settings.url.token_request
+        )
+        self.allocations_url = self.manifest_data.get("url", {}).get(
+            "allocations", settings.url.allocations
+        )
         self._access_token = None
         self._subscription_pools = None
         self._active_pools = []
@@ -64,7 +65,7 @@ def access_token(self):
             else:
                 self._access_token = token_data["access_token"]
         return self._access_token
-    
+
     @cached_property
     def valid_sat_versions(self):
         headers = {
@@ -74,11 +75,9 @@ def valid_sat_versions(self):
         valid_sat_versions = []
         sat_versions_response = simple_retry(
             self.requester.get,
-            cmd_args=[
-                    f"{self.allocations_url}/versions"
-                ],
+            cmd_args=[f"{self.allocations_url}/versions"],
             cmd_kwargs=headers,
-            ).json()
+        ).json()
         if self.is_mock:
             sat_versions_response = sat_versions_response.version_response
         for ver_dict in sat_versions_response["body"]:
@@ -100,16 +99,16 @@ def create_subscription_allocation(self):
             cmd_args=[f"{self.allocations_url}"],
             cmd_kwargs=allocation_data,
         ).json()
-        logger.debug(
-            f"Received response {self.allocation} when attempting to create allocation."
-        )
-        if ("error" in self.allocation.keys() and 
-            "invalid version" in self.allocation['error'].values()):
+        logger.debug(f"Received response {self.allocation} when attempting to create allocation.")
+        if (
+            "error" in self.allocation.keys()
+            and "invalid version" in self.allocation['error'].values()
+        ):
             raise ValueError(
-                                f"{self.sat_version} is not a valid version number."
-                                "Versions must be in the form of \"sat-X.Y\". Current"
-                                f"valid versions are {self.valid_sat_versions}."
-                            )
+                f"{self.sat_version} is not a valid version number."
+                "Versions must be in the form of \"sat-X.Y\". Current"
+                f"valid versions are {self.valid_sat_versions}."
+            )
         self.allocation_uuid = self.allocation["body"]["uuid"]
         if self.simple_content_access == "disabled":
             simple_retry(
@@ -154,9 +153,7 @@ def subscription_pools(self):
             }
             self._subscription_pools = simple_retry(
                 self.requester.get,
-                cmd_args=[
-                    f"{self.allocations_url}/{self.allocation_uuid}/pools"
-                ],
+                cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}/pools"],
                 cmd_kwargs=data,
             ).json()
             if self.is_mock:
@@ -168,9 +165,7 @@ def subscription_pools(self):
             # parameter.
             while _results == 50:
                 _offset += 50
-                logger.debug(
-                    f"Fetching additional subscription pools with an offset of {_offset}."
-                )
+                logger.debug(f"Fetching additional subscription pools with an offset of {_offset}.")
                 data = {
                     "headers": {"Authorization": f"Bearer {self.access_token}"},
                     "proxies": self.manifest_data.get("proxies", settings.proxies),
@@ -178,9 +173,7 @@ def subscription_pools(self):
                 }
                 offset_pools = simple_retry(
                     self.requester.get,
-                    cmd_args=[
-                        f"{self.allocations_url}/{self.allocation_uuid}/pools"
-                    ],
+                    cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}/pools"],
                     cmd_kwargs=data,
                 ).json()
                 if self.is_mock:
@@ -201,17 +194,13 @@ def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
         }
         add_entitlements = simple_retry(
             self.requester.post,
-            cmd_args=[
-                f"{self.allocations_url}/{self.allocation_uuid}/entitlements"
-            ],
+            cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}/entitlements"],
             cmd_kwargs=data,
         )
         return add_entitlements
 
     def verify_allocation_entitlements(self, entitlement_quantity, subscription_name):
-        logger.info(
-            f"Verifying the entitlement quantity of {subscription_name} on the allocation."
-        )
+        logger.info(f"Verifying the entitlement quantity of {subscription_name} on the allocation.")
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
             "proxies": self.manifest_data.get("proxies", settings.proxies),
@@ -232,9 +221,7 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
         logger.debug(f"Current entitlement is {current_entitlement}")
         self.attached_quantity = current_entitlement[0]["entitlementQuantity"]
         if self.attached_quantity == entitlement_quantity:
-            logger.debug(
-                f"Operation successful. Attached {self.attached_quantity} entitlements."
-            )
+            logger.debug(f"Operation successful. Attached {self.attached_quantity} entitlements.")
             return True
         elif self.attached_quantity < entitlement_quantity:
             logger.debug(
@@ -255,12 +242,12 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
             for d in subscription_pools["body"]
             if d["subscriptionName"] == subscription_data["name"]
         ]
-        logger.debug(
-            f"The following pools are matches for this subscription: {matching}"
-        )
+        logger.debug(f"The following pools are matches for this subscription: {matching}")
         for match in matching:
-            if (match["entitlementsAvailable"] > subscription_data["quantity"] or
-                match["entitlementsAvailable"] == -1):
+            if (
+                match["entitlementsAvailable"] > subscription_data["quantity"]
+                or match["entitlementsAvailable"] == -1
+            ):
                 logger.debug(
                     f"Pool {match['id']} is a match for this subscription and has "
                     f"{match['entitlementsAvailable']} entitlements available."
@@ -334,17 +321,13 @@ def trigger_manifest_export(self):
         )
         trigger_export_job = simple_retry(
             self.requester.get,
-            cmd_args=[
-                f"{self.allocations_url}/{self.allocation_uuid}/export"
-            ],
+            cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}/export"],
             cmd_kwargs=data,
         ).json()
         export_job_id = trigger_export_job["body"]["exportJobID"]
         export_job = simple_retry(
             self.requester.get,
-            cmd_args=[
-                f"{self.allocations_url}/{self.allocation_uuid}/exportJob/{export_job_id}"
-            ],
+            cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}/exportJob/{export_job_id}"],
             cmd_kwargs=data,
         )
         request_count = 1
@@ -358,9 +341,7 @@ def trigger_manifest_export(self):
                 ],
                 cmd_kwargs=data,
             )
-            logger.debug(
-                f"Attempting to export manifest. Attempt number: {request_count}"
-            )
+            logger.debug(f"Attempting to export manifest. Attempt number: {request_count}")
             if request_count > 50:
                 limit_exceeded = True
                 logger.info(
diff --git a/manifester/settings.py b/manifester/settings.py
index 37b6712..f3431f1 100644
--- a/manifester/settings.py
+++ b/manifester/settings.py
@@ -1,8 +1,7 @@
 import os
 from pathlib import Path
 
-from dynaconf import Dynaconf
-from dynaconf import Validator
+from dynaconf import Dynaconf, Validator
 
 settings_file = "manifester_settings.yaml"
 MANIFESTER_DIRECTORY = Path()
diff --git a/pyproject.toml b/pyproject.toml
new file mode 100644
index 0000000..fe4d380
--- /dev/null
+++ b/pyproject.toml
@@ -0,0 +1,62 @@
+[tool.black]
+line-length = 100
+skip-string-normalization = true
+include = '\.pyi?$'
+exclude = '''
+/(
+    \.git
+  | \.hg
+  | \.mypy_cache
+  | \.venv
+  | _build
+  | buck-out
+  | build
+  | dist
+)/
+'''
+
+[tool.ruff]
+target-version = "py311"
+fixable = ["ALL"]
+
+select = [
+    # "C90", # mccabe
+    "E",  # pycodestyle
+    "F",  # flake8
+    "I", # isort
+    # "Q",  # flake8-quotes
+    "PT",  # flake8-pytest
+    "UP",  # pyupgrade
+    "W",  # pycodestyle
+]
+
+ignore = [
+    "E501",  # line too long - handled by black
+    "PT004", # pytest underscrore prefix for non-return fixtures
+    "PT005", # pytest no underscrore prefix for return fixtures
+    "PT011",
+]
+
+[tool.ruff.isort]
+force-sort-within-sections = true
+known-first-party = [
+    "manifester",
+]
+combine-as-imports = true
+
+[tool.ruff.flake8-pytest-style]
+fixture-parentheses = false
+mark-parentheses = false
+
+[tool.ruff.flake8-quotes]
+inline-quotes = "single"
+
+[tool.ruff.mccabe]
+max-complexity = 20
+
+[tool.ruff.per-file-ignores]
+"manifester/__init__.py" = ["F401"]
+
+[tool.pytest.ini_options]
+junit_logging = 'all'
+addopts = '--show-capture=no'
diff --git a/setup.py b/setup.py
index 42eaec6..ffe0d37 100644
--- a/setup.py
+++ b/setup.py
@@ -1,7 +1,6 @@
 #!/usr/bin/env python
 from setuptools import setup
 
-
 setup(
     use_scm_version=True,
 )
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 208064e..70628fc 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -1,15 +1,13 @@
-from unittest.mock import Mock
-
 from functools import cached_property
-from requests import request
-from manifester import Manifester
-from manifester.settings import settings
-from manifester.helpers import MockStub, fake_http_response_code
-import pytest
 import random
 import string
 import uuid
 
+import pytest
+
+from manifester import Manifester
+from manifester.helpers import MockStub, fake_http_response_code
+
 manifest_data = {
     "log_level": "debug",
     "offline_token": "test",
@@ -21,20 +19,19 @@
     "sat_version": "sat-6.14",
     "subscription_data": [
         {
-            "name": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)", 
+            "name": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)",
             "quantity": 1,
-            },
+        },
         {
             "name": "Red Hat Satellite Infrastructure Subscription",
-            "quantity": 1
+            "quantity": 1,
         },
     ],
     "simple_content_access": "enabled",
 }
 
 sub_pool_response = {
-    'body':
-    [
+    'body': [
         {
             'id': f'{uuid.uuid4().hex}',
             'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
@@ -57,8 +54,9 @@
         },
     ],
 }
-class RhsmApiStub(MockStub):
 
+
+class RhsmApiStub(MockStub):
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
@@ -84,14 +82,16 @@ def post(self, *args, **kwargs):
             return self
 
     def get(self, *args, **kwargs):
-        """"Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
+        """ "Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
 
         if args[0].endswith("versions"):
-            self.version_response = {'body': [
-                {'value': 'sat-6.14', 'description': 'Satellite 6.14'},
-                {'value': 'sat-6.13', 'description': 'Satellite 6.13'},
-                {'value': 'sat-6.12', 'description': 'Satellite 6.12'},
-            ]}
+            self.version_response = {
+                'body': [
+                    {'value': 'sat-6.14', 'description': 'Satellite 6.14'},
+                    {'value': 'sat-6.13', 'description': 'Satellite 6.13'},
+                    {'value': 'sat-6.12', 'description': 'Satellite 6.12'},
+                ]
+            }
             return self
         if args[0].endswith("pools") and not self._has_offset:
             self.pool_response = sub_pool_response
@@ -100,11 +100,13 @@ def get(self, *args, **kwargs):
             if kwargs["params"]["offset"] != 50:
                 self.pool_response = {'body': []}
                 for x in range(50):
-                    self.pool_response["body"].append({
-                        'id': f'{"".join(random.sample(string.ascii_letters, 12))}', 
-                        'subscriptionName': 'Red Hat Satellite Infrastructure Subscription', 
-                        'entitlementsAvailable': random.randrange(100),
-                        })
+                    self.pool_response["body"].append(
+                        {
+                            'id': f'{"".join(random.sample(string.ascii_letters, 12))}',
+                            'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
+                            'entitlementsAvailable': random.randrange(100),
+                        }
+                    )
                 return self
             else:
                 self.pool_response["body"] += sub_pool_response["body"]
@@ -136,17 +138,21 @@ def delete(self, *args, **kwargs):
         if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
             del self.status_code
             self.content = b""
-            self._good_codes=[204]
+            self._good_codes = [204]
             return self
 
 
 def test_basic_init(manifest_category="golden_ticket"):
-    """Test that manifester can initialize with the minimum required arguments and verify that resulting object has an access token attribute"""
+    """Test that manifester can initialize with the minimum required arguments and verify that
+    resulting object has an access token attribute"""
 
-    manifester_inst = Manifester(manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None))
+    manifester_inst = Manifester(
+        manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None)
+    )
     assert isinstance(manifester_inst, Manifester)
     assert manifester_inst.access_token == "this is a simulated access token"
 
+
 def test_create_allocation():
     """Test that manifester's create_subscription_allocation method returns a UUID"""
 
@@ -154,22 +160,30 @@ def test_create_allocation():
     allocation_uuid = manifester.create_subscription_allocation()
     assert allocation_uuid.uuid == "1234567890"
 
+
 def test_negative_simple_retry_timeout():
     """Test that exceeding the attempt limit when retrying a failed API call results in an exception"""
 
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=0))
+    manifester = Manifester(
+        manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=0)
+    )
     manifester.requester._fail_rate = 100
     with pytest.raises(Exception) as exception:
         manifester.get_manifest()
     assert str(exception.value) == "Retry timeout exceeded"
 
+
 def test_negative_manifest_export_timeout():
     """Test that exceeding the attempt limit when exporting a manifest results in an exception"""
 
     with pytest.raises(Exception) as exception:
-        manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict={"force_export_failure": True}))
+        Manifester(
+            manifest_category="golden_ticket",
+            requester=RhsmApiStub(in_dict={"force_export_failure": True}),
+        )
     assert str(exception.value) == "Export timeout exceeded"
 
+
 def test_get_manifest():
     """Test that manifester's get_manifest method returns a manifest"""
 
@@ -178,6 +192,7 @@ def test_get_manifest():
     assert manifest.content.decode("utf-8") == "this is a simulated manifest"
     assert manifest.status_code == 200
 
+
 def test_delete_subscription_allocation():
     """Test that manifester's delete_subscription_allocation method deletes a subscription allocation"""
 
@@ -187,6 +202,7 @@ def test_delete_subscription_allocation():
     assert response.status_code == 204
     assert response.content == b""
 
+
 def test_ingest_manifest_data_via_dict():
     """Test that manifester is able to read configuration data from a dictionary"""
 
@@ -197,25 +213,34 @@ def test_ingest_manifest_data_via_dict():
     assert manifester.allocations_url == manifest_data["url"]["allocations"]
     assert manifester.sat_version == manifest_data["sat_version"]
 
+
 def test_get_subscription_pools_with_offset():
-    """Tests that manifester can retrieve all subscription pools from an account containing more than 50 pools"""
+    """Tests that manifester can retrieve all subscription pools from an account containing
+    more than 50 pools
+    """
 
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True))
+    manifester = Manifester(
+        manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True)
+    )
     manifester.get_manifest()
     assert len(manifester.subscription_pools["body"]) > 50
 
+
 def test_correct_subs_added_to_allocation():
-    """Test that the subscriptions added to the subscription allocation match the subscription data defined in manifester's config"""
+    """Test that the subscriptions added to the subscription allocation match the subscription data
+    defined in manifester's config
+    """
 
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
-    active_subs = sorted([ x["subscriptionName"] for x in manifester._active_pools ])
-    sub_names_from_config = sorted([ x["NAME"] for x in manifester.subscription_data ])
+    active_subs = sorted([x["subscriptionName"] for x in manifester._active_pools])
+    sub_names_from_config = sorted([x["NAME"] for x in manifester.subscription_data])
     assert active_subs == sub_names_from_config
 
+
 def test_invalid_sat_version():
     """Test that an invalid sat_version value will be replaced with the latest valid sat_version"""
 
     manifest_data["sat_version"] = "sat-6.20"
     manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
-    assert manifester.sat_version == "sat-6.14"
\ No newline at end of file
+    assert manifester.sat_version == "sat-6.14"

From 55f3cda3d877192f57648cdfccdead333629c727 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Mon, 4 Dec 2023 11:52:13 -0500
Subject: [PATCH 20/26] Switch to ruff for linting/formatting

This commit switches from using black for formatting and ruff for
linting to using ruff for both of these. It also implements a more
complete pyproject.toml and includes many small changes to get the
project into a passing state for ruff. Additionally, it removes setup.py
and setup.cfg as these are now redundant with pyproject.toml.
---
 .github/workflows/codeql-analysis.yml         |  16 +-
 .github/workflows/python-publish.yml          |   8 +-
 .github/workflows/update_manifester_image.yml |   8 +-
 .pre-commit-config.yaml                       |   9 +-
 manifester/commands.py                        |   3 +
 manifester/helpers.py                         |  28 ++--
 manifester/logger.py                          |  13 +-
 manifester/manifester.py                      |  72 +++++---
 manifester/settings.py                        |   1 +
 pyproject.toml                                | 155 ++++++++++++++++--
 setup.cfg                                     |  42 -----
 setup.py                                      |   6 -
 tests/test_manifester.py                      |  97 +++++------
 13 files changed, 275 insertions(+), 183 deletions(-)
 delete mode 100644 setup.cfg
 delete mode 100644 setup.py

diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
index 2a8d60a..609cdea 100644
--- a/.github/workflows/codeql-analysis.yml
+++ b/.github/workflows/codeql-analysis.yml
@@ -5,6 +5,10 @@ on:
     branches: [master]
   pull_request:
     types: [opened, synchronize, reopened]
+    paths-ignore:
+        - "*.md"
+        - "*.example"
+        - ".gitignore"
 
 jobs:
   analyze:
@@ -14,22 +18,22 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.8", "3.9", "3.10"]
+        python-version: ["3.10", "3.11", "3.12"]
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
 
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v1
+      uses: github/codeql-action/init@v2
       with:
         languages: ${{ matrix.language }}
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v1
+      uses: github/codeql-action/analyze@v2
 
     - name: Setup Python
-      uses: actions/setup-python@v2
+      uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python-version }}
 
@@ -39,4 +43,4 @@ jobs:
         pip install -U .[test]
         cp manifester_settings.yaml.example manifester_settings.yaml
         manifester --help
-        # pytest -v tests/ --ignore tests/functional
+        pytest -v tests/
diff --git a/.github/workflows/python-publish.yml b/.github/workflows/python-publish.yml
index 73b8408..5a261ba 100644
--- a/.github/workflows/python-publish.yml
+++ b/.github/workflows/python-publish.yml
@@ -12,12 +12,12 @@ jobs:
     strategy:
       matrix:
         # build/push in lowest support python version
-        python-version: [ 3.8 ]
+        python-version: [ 3.10 ]
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
 
-    - uses: actions/setup-python@v2
+    - uses: actions/setup-python@v4
       with:
         python-version: ${{ matrix.python-version }}
 
@@ -29,7 +29,7 @@ jobs:
         python -m twine check dist/*
 
     - name: Build and publish
-      uses: pypa/gh-action-pypi-publish@release/v1
+      uses: pypa/gh-action-pypi-publish@v1.8.11
       with:
         user: __token__
         password: ${{ secrets.PYPI_TOKEN }}
diff --git a/.github/workflows/update_manifester_image.yml b/.github/workflows/update_manifester_image.yml
index a92d385..3f43f64 100644
--- a/.github/workflows/update_manifester_image.yml
+++ b/.github/workflows/update_manifester_image.yml
@@ -11,16 +11,16 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
       - name: Set up QEMU
-        uses: docker/setup-qemu-action@v1
+        uses: docker/setup-qemu-action@v3
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v1
+        uses: docker/setup-buildx-action@v3
 
       - name: Login to Quay Container Registry
-        uses: docker/login-action@v1
+        uses: docker/login-action@v3
         with:
           registry: ${{ secrets.QUAY_SERVER }}
           username: ${{ secrets.QUAY_USERNAME }}
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 731998e..7980a70 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -7,16 +7,13 @@ repos:
     - id: trailing-whitespace
     - id: check-yaml
     - id: debug-statements
-  - repo: https://github.com/psf/black
-    rev: 22.10.0
-    hooks:
-    - id: black
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.0.277
+    rev: v0.1.6
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]
+      - id: ruff-format
   - repo: https://github.com/gitleaks/gitleaks
     rev: v8.18.0
     hooks:
-      - id: gitleaks
\ No newline at end of file
+      - id: gitleaks
diff --git a/manifester/commands.py b/manifester/commands.py
index c5e6b98..00e6bbd 100644
--- a/manifester/commands.py
+++ b/manifester/commands.py
@@ -1,3 +1,4 @@
+"""Defines the CLI commands for Manifester."""
 import click
 
 from manifester import Manifester
@@ -6,6 +7,7 @@
 # To do: add a command for returning subscription pools
 @click.group
 def cli():
+    """Command-line interface for manifester."""
     pass
 
 
@@ -17,6 +19,7 @@ def cli():
 )
 @click.option("--allocation_name", type=str, help="Name of upstream subscription allocation")
 def get_manifest(manifest_category, allocation_name):
+    """Return a subscription manifester based on the settings for the provided manifest_category."""
     manifester = Manifester(manifest_category, allocation_name)
     manifester.create_subscription_allocation()
     for sub in manifester.subscription_data:
diff --git a/manifester/helpers.py b/manifester/helpers.py
index 22473b2..d202a1b 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -1,3 +1,4 @@
+"""Defines helper functions used by Manifester."""
 from collections import UserDict
 import random
 import time
@@ -6,8 +7,7 @@
 
 
 def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_timeout=1):
-    """Re(Try) a function given its args and kwargs up until a max timeout"""
-
+    """Re(Try) a function given its args and kwargs up until a max timeout."""
     cmd_args = cmd_args if cmd_args else []
     cmd_kwargs = cmd_kwargs if cmd_kwargs else {}
     # If additional debug information is needed, the following log entry can be modified to
@@ -27,30 +27,29 @@ def simple_retry(cmd, cmd_args=None, cmd_kwargs=None, max_timeout=240, _cur_time
 
 
 def process_sat_version(sat_version, valid_sat_versions):
-    """Ensure that the sat_version parameter is properly formatted for the RHSM API when creating
-    a subscription allocation with the 'POST allocations' endpoint"""
+    """Ensure that the sat_version parameter is properly formatted for the RHSM API."""
+    expected_length = 8
     if sat_version not in valid_sat_versions:
         # The valid values for the sat_version parameter when creating a subscription allocation
         # are all 8 characters or less (e.g. 'sat-6.11'). Some data sources may include a Z-stream
         # version (e.g. 'sat-6.11.0') when retrieving this value from settings. The conditional
         # below assumes that, if the length of sat_version is greated than 8 characters, it includes
         # a Z-stream version that should be removed.
-        if len(sat_version) > 8:
-            sat_version = sat_version.split('.')
+        if len(sat_version) > expected_length:
+            sat_version = sat_version.split(".")
             sat_version = sat_version[0:2]
             sat_version = ".".join(sat_version)
         # If sat_version is still not valid, default to the latest valid version.
         if sat_version not in valid_sat_versions:
             valid_sat_versions.sort(
-                key=lambda i: int(i.split('-')[-1].split('.')[-1]), reverse=True
+                key=lambda i: int(i.split("-")[-1].split(".")[-1]), reverse=True
             )
             return valid_sat_versions[0]
     return sat_version
 
 
 def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
-    """Return an HTTP response code randomly selected from sets of good and bad codes"""
-
+    """Return an HTTP response code randomly selected from sets of good and bad codes."""
     if random.random() > (fail_rate / 100):
         return random.choice(good_codes)
     else:
@@ -58,10 +57,10 @@ def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
 
 
 class MockStub(UserDict):
-    """Test helper class. Allows for both arbitrary mocking and stubbing"""
+    """Test helper class. Allows for both arbitrary mocking and stubbing."""
 
     def __init__(self, in_dict=None):
-        """Initialize the class and all nested dictionaries"""
+        """Initialize the class and all nested dictionaries."""
         if in_dict is None:
             in_dict = {}
         for key, value in in_dict.items():
@@ -78,9 +77,15 @@ def __init__(self, in_dict=None):
         super().__init__(in_dict)
 
     def __getattr__(self, name):
+        """Fallback to returning self if attribute doesn't exist."""
         return self
 
     def __getitem__(self, key):
+        """Get an item from the dictionary-like object.
+
+        If the key is a string, this method will attempt to get an attribute with that name.
+        If the key is not found, this method will return the object itself.
+        """
         if isinstance(key, str):
             item = getattr(self, key, self)
         try:
@@ -90,4 +95,5 @@ def __getitem__(self, key):
         return item
 
     def __call__(self, *args, **kwargs):
+        """Allow MockStub to be used like a function."""
         return self
diff --git a/manifester/logger.py b/manifester/logger.py
index ef85224..3926201 100644
--- a/manifester/logger.py
+++ b/manifester/logger.py
@@ -1,3 +1,4 @@
+"""Defines manifester's internal logging."""
 import logging
 from pathlib import Path
 
@@ -7,26 +8,22 @@
 
 
 def setup_logzero(level="info", path="logs/manifester.log", silent=True):
+    """Call logzero setup with the given settings."""
     Path(path).parent.mkdir(parents=True, exist_ok=True)
     log_fmt = "%(color)s[%(levelname)s %(asctime)s]%(end_color)s %(message)s"
     debug_fmt = (
-        "%(color)s[%(levelname)1.1s %(asctime)s %(module)s:%(lineno)d]"
-        "%(end_color)s %(message)s"
+        "%(color)s[%(levelname)1.1s %(asctime)s %(module)s:%(lineno)d]%(end_color)s %(message)s"
     )
     log_level = getattr(logging, level.upper(), logging.INFO)
     # formatter for terminal
-    formatter = logzero.LogFormatter(
-        fmt=debug_fmt if log_level is logging.DEBUG else log_fmt
-    )
+    formatter = logzero.LogFormatter(fmt=debug_fmt if log_level is logging.DEBUG else log_fmt)
     logzero.setup_default_logger(formatter=formatter, disableStderrLogger=silent)
     logzero.loglevel(log_level)
     # formatter for file
     formatter = logzero.LogFormatter(
         fmt=debug_fmt if log_level is logging.DEBUG else log_fmt, color=False
     )
-    logzero.logfile(
-        path, loglevel=log_level, maxBytes=1e9, backupCount=3, formatter=formatter
-    )
+    logzero.logfile(path, loglevel=log_level, maxBytes=1e9, backupCount=3, formatter=formatter)
 
 
 setup_logzero(level=settings.get("log_level", "info"))
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 4d55518..132661a 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -1,3 +1,8 @@
+"""Main interface for the RHSM API.
+
+This module defines the `Manifester` class, which provides methods for authenticating to and
+interacting with the RHSM Subscription API for the purpose of generating a subscription manifest.
+"""
 from functools import cached_property
 from pathlib import Path
 import random
@@ -5,12 +10,15 @@
 
 from dynaconf.utils.boxing import DynaBox
 from logzero import logger
+from requests.exceptions import Timeout
 
 from manifester.helpers import process_sat_version, simple_retry
 from manifester.settings import settings
 
 
 class Manifester:
+    """Main Manifester class responsible for generating a manifest from the provided settings."""
+
     def __init__(self, manifest_category, allocation_name=None, **kwargs):
         if isinstance(manifest_category, dict):
             self.manifest_data = DynaBox(manifest_category)
@@ -25,7 +33,7 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
             self.requester = requests
             self.is_mock = False
         self.allocation_name = allocation_name or "".join(random.sample(string.ascii_letters, 10))
-        self.manifest_name = Path(f'{self.allocation_name}_manifest.zip')
+        self.manifest_name = Path(f"{self.allocation_name}_manifest.zip")
         self.offline_token = kwargs.get("offline_token", self.manifest_data.offline_token)
         self.subscription_data = self.manifest_data.subscription_data
         self.token_request_data = {
@@ -52,6 +60,10 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
 
     @property
     def access_token(self):
+        """Representation of an RHSM API access token.
+
+        Used to authenticate requests to the RHSM API.
+        """
         if not self._access_token:
             token_request_data = {"data": self.token_request_data}
             logger.debug("Generating access token")
@@ -68,11 +80,11 @@ def access_token(self):
 
     @cached_property
     def valid_sat_versions(self):
+        """Retrieves the list of valid Satellite versions from the RHSM API."""
         headers = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
             "proxies": self.manifest_data.get("proxies", settings.proxies),
         }
-        valid_sat_versions = []
         sat_versions_response = simple_retry(
             self.requester.get,
             cmd_args=[f"{self.allocations_url}/versions"],
@@ -80,11 +92,11 @@ def valid_sat_versions(self):
         ).json()
         if self.is_mock:
             sat_versions_response = sat_versions_response.version_response
-        for ver_dict in sat_versions_response["body"]:
-            valid_sat_versions.append(ver_dict["value"])
+        valid_sat_versions = [ver_dict["value"] for ver_dict in sat_versions_response["body"]]
         return valid_sat_versions
 
     def create_subscription_allocation(self):
+        """Creates a new consumer in the provided RHSM account and returns its UUID."""
         allocation_data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
             "proxies": self.manifest_data.get("proxies", settings.proxies),
@@ -100,15 +112,6 @@ def create_subscription_allocation(self):
             cmd_kwargs=allocation_data,
         ).json()
         logger.debug(f"Received response {self.allocation} when attempting to create allocation.")
-        if (
-            "error" in self.allocation.keys()
-            and "invalid version" in self.allocation['error'].values()
-        ):
-            raise ValueError(
-                f"{self.sat_version} is not a valid version number."
-                "Versions must be in the form of \"sat-X.Y\". Current"
-                f"valid versions are {self.valid_sat_versions}."
-            )
         self.allocation_uuid = self.allocation["body"]["uuid"]
         if self.simple_content_access == "disabled":
             simple_retry(
@@ -127,6 +130,7 @@ def create_subscription_allocation(self):
         return self.allocation_uuid
 
     def delete_subscription_allocation(self):
+        """Deletes the specified subscription allocation and returns the RHSM API's response."""
         self._access_token = None
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
@@ -144,6 +148,11 @@ def delete_subscription_allocation(self):
 
     @property
     def subscription_pools(self):
+        """Fetches the list of subscription pools from account.
+
+        Returns a list of dictionaries containing metadata from the pools.
+        """
+        max_results_per_page = 50
         if not self._subscription_pools:
             _offset = 0
             data = {
@@ -163,7 +172,7 @@ def subscription_pools(self):
             # For organizations with more than 50 subscription pools, the loop below works around
             # this limit by repeating calls with a progressively larger value for the `offset`
             # parameter.
-            while _results == 50:
+            while _results == max_results_per_page:
                 _offset += 50
                 logger.debug(f"Fetching additional subscription pools with an offset of {_offset}.")
                 data = {
@@ -187,6 +196,7 @@ def subscription_pools(self):
         return self._subscription_pools
 
     def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
+        """Attempts to add the set of subscriptions defined in the settings to the allocation."""
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
             "proxies": self.manifest_data.get("proxies", settings.proxies),
@@ -200,6 +210,7 @@ def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
         return add_entitlements
 
     def verify_allocation_entitlements(self, entitlement_quantity, subscription_name):
+        """Checks that the entitlements in the allocation match those defined in settings."""
         logger.info(f"Verifying the entitlement quantity of {subscription_name} on the allocation.")
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
@@ -236,6 +247,12 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
             return True
 
     def process_subscription_pools(self, subscription_pools, subscription_data):
+        """Loops through the list of subscription pools in the account.
+
+        Identifies pools that match the subscription names and quantities defined in settings, then
+        attempts to add the specified quantity of each subscription to the allocation.
+        """
+        success_code = 200
         logger.debug(f"Finding a matching pool for {subscription_data['name']}.")
         matching = [
             d
@@ -295,7 +312,7 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
                         )
                         self._active_pools.append(match)
                         break
-                elif add_entitlements.status_code == 200:
+                elif add_entitlements.status_code == success_code:
                     logger.debug(
                         f"Successfully added {subscription_data['quantity']} entitlements of "
                         f"{subscription_data['name']} to the allocation."
@@ -303,17 +320,23 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
                     self._active_pools.append(match)
                     break
                 else:
-                    raise Exception(
+                    raise RuntimeError(
                         "Something went wrong while adding entitlements. Received response status "
                         f"{add_entitlements.status_code}."
                     )
 
     def trigger_manifest_export(self):
+        """Triggers job to export manifest from subscription allocation.
+
+        Starts the export job, monitors the status of the job, and downloads the manifest on
+        successful completion of the job.
+        """
+        max_requests = 50
+        success_code = 200
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
             "proxies": self.manifest_data.get("proxies", settings.proxies),
         }
-        # Should this use the XDG Base Directory Specification?
         local_file = Path(f"manifests/{self.manifest_name}")
         local_file.parent.mkdir(parents=True, exist_ok=True)
         logger.info(
@@ -332,8 +355,7 @@ def trigger_manifest_export(self):
         )
         request_count = 1
         limit_exceeded = False
-        while export_job.status_code != 200:
-            print(export_job.status_code)
+        while export_job.status_code != success_code:
             export_job = simple_retry(
                 self.requester.get,
                 cmd_args=[
@@ -342,14 +364,13 @@ def trigger_manifest_export(self):
                 cmd_kwargs=data,
             )
             logger.debug(f"Attempting to export manifest. Attempt number: {request_count}")
-            if request_count > 50:
+            if request_count > max_requests:
                 limit_exceeded = True
                 logger.info(
                     "Manifest export job status check limit exceeded. This may indicate an "
                     "upstream issue with Red Hat Subscription Management."
                 )
-                raise Exception("Export timeout exceeded")
-                break
+                raise Timeout("Export timeout exceeded")
             request_count += 1
         if limit_exceeded:
             self.content = None
@@ -374,6 +395,11 @@ def trigger_manifest_export(self):
         return manifest
 
     def get_manifest(self):
+        """Provides a subscription manifest based on settings.
+
+        Calls the methods required to create a new subscription allocation, add the appropriate
+        subscriptions to the allocation, export a manifest, and download the manifest.
+        """
         self.create_subscription_allocation()
         for sub in self.subscription_data:
             self.process_subscription_pools(
@@ -383,6 +409,7 @@ def get_manifest(self):
         return self.trigger_manifest_export()
 
     def __enter__(self):
+        """Generates and returns a manifest."""
         try:
             return self.get_manifest()
         except:
@@ -390,4 +417,5 @@ def __enter__(self):
             raise
 
     def __exit__(self, *tb_args):
+        """Deletes subscription allocation on teardown."""
         self.delete_subscription_allocation()
diff --git a/manifester/settings.py b/manifester/settings.py
index f3431f1..ba83d1f 100644
--- a/manifester/settings.py
+++ b/manifester/settings.py
@@ -1,3 +1,4 @@
+"""Retrieves settings from configuration file and runs Dynaconf validators."""
 import os
 from pathlib import Path
 
diff --git a/pyproject.toml b/pyproject.toml
index fe4d380..b5218d9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -14,29 +14,155 @@ exclude = '''
   | dist
 )/
 '''
+[build-system]
+requires = ["setuptools", "setuptools-scm[toml]", "wheel"]
+build-backend = "setuptools.build_meta"
+
+[project]
+name = "manifester"
+description = "Red Hat subscriptions made manifest."
+readme = "README.md"
+requires-python = ">=3.10"
+license = {file = "LICENSE", name = "Apache License Version 2.0"}
+keywords = ["manifester", "RHSM"]
+authors = [
+    {name = "Danny Synk", email = "dsynk@redhat.com"}
+]
+classifiers = [
+    "Development Status :: 4 - Beta",
+    "Intended Audience :: Developers",
+    "License :: OSI Approved :: Apache Software License",
+    "Natural Language :: English",
+    "Programming Language :: Python :: 3",
+    "Programming Language :: Python :: 3.10",
+    "Programming Language :: Python :: 3.11",
+]
+dependencies = [
+    "click",
+    "dynaconf",
+    "logzero",
+    "pyyaml",
+    "requests",
+    "setuptools",
+]
+dynamic = ["version"]
+
+[project.urls]
+Repository = "https://github.com/SatelliteQE/manifester"
+
+[project.optional-dependencies]
+dev = [
+    "pre-commit",
+    "pytest",
+    "ruff",
+]
+setup = [
+    "build",
+    "twine",
+]
+
+[tools.setuptools]
+platforms = ["any"]
+zip-safe = false
+include-package-data = true
+
+[tool.setuptools.packages.find]
+include = ["manifester"]
+
+[tool.setuptools_scm]
+
+[tool.pytest.ini_options]
+testpaths = ["tests"]
 
 [tool.ruff]
+line-length = 100
 target-version = "py311"
 fixable = ["ALL"]
 
 select = [
-    # "C90", # mccabe
-    "E",  # pycodestyle
-    "F",  # flake8
+    "B002", # Python does not support the unary prefix increment
+    "B007", # Loop control variable {name} not used within loop body
+    "B009", # Do not call getattr with a constant attribute value
+    "B010", # Do not call setattr with a constant attribute value
+    "B011", # Do not `assert False`, raise `AssertionError` instead
+    "B013", # Redundant tuple in exception handler
+    "B014", # Exception handler with duplicate exception
+    "B023", # Function definition does not bind loop variable {name}
+    "B026", # Star-arg unpacking after a keyword argument is strongly discouraged
+    "BLE001", # Using bare except clauses is prohibited
+    "C", # complexity
+    "C4", # flake8-comprehensions
+    "COM818", # Trailing comma on bare tuple prohibited
+    "D", # docstrings
+    "E", # pycodestyle
+    "F", # pyflakes/autoflake
+    "G", # flake8-logging-format
     "I", # isort
-    # "Q",  # flake8-quotes
-    "PT",  # flake8-pytest
-    "UP",  # pyupgrade
-    "W",  # pycodestyle
+    "ISC001", # Implicitly concatenated string literals on one line
+    "N804", # First argument of a class method should be named cls
+    "N805", # First argument of a method should be named self
+    "N815", # Variable {name} in class scope should not be mixedCase
+    "N999", # Invalid module name: '{name}'
+    "PERF", # Perflint rules
+    "PGH004", # Use specific rule codes when using noqa
+    "PLC0414", # Useless import alias. Import alias does not rename original package.
+    "PLC", # pylint
+    "PLE", # pylint
+    "PLR", # pylint
+    "PLW", # pylint
+    "PTH", # Use pathlib
+    "RUF", # Ruff-specific rules
+    "S103", # bad-file-permissions
+    "S108", # hardcoded-temp-file
+    "S110", # try-except-pass
+    "S112", # try-except-continue
+    "S113", # Probable use of requests call without timeout
+    "S306", # suspicious-mktemp-usage
+    "S307", # suspicious-eval-usage
+    "S601", # paramiko-call
+    "S602", # subprocess-popen-with-shell-equals-true
+    "S604", # call-with-shell-equals-true
+    "S609", # unix-command-wildcard-injection
+    "SIM105", # Use contextlib.suppress({exception}) instead of try-except-pass
+    "SIM117", # Merge with-statements that use the same scope
+    "SIM118", # Use {key} in {dict} instead of {key} in {dict}.keys()
+    "SIM201", # Use {left} != {right} instead of not {left} == {right}
+    "SIM208", # Use {expr} instead of not (not {expr})
+    "SIM212", # Use {a} if {a} else {b} instead of {b} if not {a} else {a}
+    "SIM300", # Yoda conditions. Use 'age == 42' instead of '42 == age'.
+    "SIM401", # Use get from dict with default instead of an if block
+    "T100", # Trace found: {name} used
+    "T20", # flake8-print
+    "TRY004", # Prefer TypeError exception for invalid type
+    "TRY200", # Use raise from to specify exception cause
+    "TRY302", # Remove exception handler; error is immediately re-raised
+    "PLR0911", # Too many return statements ({returns} > {max_returns})
+    "PLR0912", # Too many branches ({branches} > {max_branches})
+    "PLR0915", # Too many statements ({statements} > {max_statements})
+    "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable
+    "PLW2901", # Outer {outer_kind} variable {name} overwritten by inner {inner_kind} target
+    "UP", # pyupgrade
+    "W", # pycodestyle
 ]
 
 ignore = [
-    "E501",  # line too long - handled by black
-    "PT004", # pytest underscrore prefix for non-return fixtures
-    "PT005", # pytest no underscrore prefix for return fixtures
-    "PT011",
+    "ANN", # flake8-annotations
+    "PGH001", # No builtin eval() allowed
+    "D203", # 1 blank line required before class docstring
+    "D213", # Multi-line docstring summary should start at the second line
+    "D406", # Section name should end with a newline
+    "D407", # Section name underlining
+    "E731", # do not assign a lambda expression, use a def
+    "PLR0913", # Too many arguments to function call ({c_args} > {max_args})
+    "RUF012", # Mutable class attributes should be annotated with typing.ClassVar
+    "D107", # Missing docstring in __init__
 ]
 
+[tool.ruff.per-file-ignores]
+"manifester/__init__.py" = ["D104", "F401",]
+"manifester/manifester.py" = ["D401",]
+"tests/test_manifester.py" = ["D100", "E501", "PLR0911", "PLR2004",]
+
 [tool.ruff.isort]
 force-sort-within-sections = true
 known-first-party = [
@@ -53,10 +179,3 @@ inline-quotes = "single"
 
 [tool.ruff.mccabe]
 max-complexity = 20
-
-[tool.ruff.per-file-ignores]
-"manifester/__init__.py" = ["F401"]
-
-[tool.pytest.ini_options]
-junit_logging = 'all'
-addopts = '--show-capture=no'
diff --git a/setup.cfg b/setup.cfg
deleted file mode 100644
index 752caee..0000000
--- a/setup.cfg
+++ /dev/null
@@ -1,42 +0,0 @@
-[metadata]
-name = manifester
-description = Manifester dynamically generates subscription manifests using the Red Hat Subscription Managament API.
-long_description = file: README.md
-long_description_content_type = text/markdown
-author = Danny Synk
-author_email = dsynk@redhat.com
-url = https://github.com/SatelliteQE/manifester
-license = Apache
-keywords = rhsm, red hat
-classifiers =
-    Development Status :: 3 - Alpha
-    Intended Audience :: Developers
-    License :: OSI Approved :: GNU General Public License v3 (GPLv3)
-    Natural Language :: English
-    Programming Language :: Python :: 3
-    Programming Language :: Python :: 3.8
-    Programming Language :: Python :: 3.9
-    Programming Language :: Python :: 3.10
-
-[options]
-install_requires =
-    click
-    dynaconf>=3.1.0
-    logzero
-    pyyaml
-    requests
-    setuptools
-packages = find:
-zip_safe = False
-
-[options.extras_require]
-test = pytest
-setup =
-    setuptools
-    setuptools-scm
-    wheel
-    twine
-
-[options.entry_points]
-console_scripts =
-    manifester = manifester.commands:cli
diff --git a/setup.py b/setup.py
deleted file mode 100644
index ffe0d37..0000000
--- a/setup.py
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/usr/bin/env python
-from setuptools import setup
-
-setup(
-    use_scm_version=True,
-)
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 70628fc..00e453e 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -11,7 +11,7 @@
 manifest_data = {
     "log_level": "debug",
     "offline_token": "test",
-    "proxies": {"https:" ""},
+    "proxies": {"https:"},
     "url": {
         "token_request": "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
         "allocations": "https://api.access.redhat.com/management/v1/allocations",
@@ -31,32 +31,34 @@
 }
 
 sub_pool_response = {
-    'body': [
+    "body": [
         {
-            'id': f'{uuid.uuid4().hex}',
-            'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
-            'entitlementsAvailable': 8,
+            "id": f"{uuid.uuid4().hex}",
+            "subscriptionName": "Red Hat Satellite Infrastructure Subscription",
+            "entitlementsAvailable": 8,
         },
         {
-            'id': f'{uuid.uuid4().hex}',
-            'subscriptionName': 'Red Hat Enterprise Linux for Virtual Datacenters, Premium',
-            'entitlementsAvailable': 8,
+            "id": f"{uuid.uuid4().hex}",
+            "subscriptionName": "Red Hat Enterprise Linux for Virtual Datacenters, Premium",
+            "entitlementsAvailable": 8,
         },
         {
-            'id': f'{uuid.uuid4().hex}',
-            'subscriptionName': 'Red Hat Beta Access',
-            'entitlementsAvailable': 8,
+            "id": f"{uuid.uuid4().hex}",
+            "subscriptionName": "Red Hat Beta Access",
+            "entitlementsAvailable": 8,
         },
         {
-            'id': f'{uuid.uuid4().hex}',
-            'subscriptionName': 'Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)',
-            'entitlementsAvailable': 8,
+            "id": f"{uuid.uuid4().hex}",
+            "subscriptionName": "Red Hat Enterprise Linux Server, Premium (Physical or Virtual Nodes)",
+            "entitlementsAvailable": 8,
         },
     ],
 }
 
 
 class RhsmApiStub(MockStub):
+    """Returns mock responses for RHSM API endpoints related to creating manifests."""
+
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
@@ -66,11 +68,11 @@ def __init__(self, in_dict=None, **kwargs):
 
     @cached_property
     def status_code(self):
+        """HTTP response code of current request."""
         return fake_http_response_code(self._good_codes, self._bad_codes, self._fail_rate)
 
     def post(self, *args, **kwargs):
-        """Simulate responses to POST requests for RHSM API endpoints used by Manifester"""
-
+        """Simulate responses to POST requests for RHSM API endpoints used by Manifester."""
         if args[0].endswith("openid-connect/token"):
             self.access_token = "this is a simulated access token"
             return self
@@ -82,14 +84,13 @@ def post(self, *args, **kwargs):
             return self
 
     def get(self, *args, **kwargs):
-        """ "Simulate responses to GET requests for RHSM API endpoints used by Manifester"""
-
+        """Simulate responses to GET requests for RHSM API endpoints used by Manifester."""
         if args[0].endswith("versions"):
             self.version_response = {
-                'body': [
-                    {'value': 'sat-6.14', 'description': 'Satellite 6.14'},
-                    {'value': 'sat-6.13', 'description': 'Satellite 6.13'},
-                    {'value': 'sat-6.12', 'description': 'Satellite 6.12'},
+                "body": [
+                    {"value": "sat-6.14", "description": "Satellite 6.14"},
+                    {"value": "sat-6.13", "description": "Satellite 6.13"},
+                    {"value": "sat-6.12", "description": "Satellite 6.12"},
                 ]
             }
             return self
@@ -98,13 +99,13 @@ def get(self, *args, **kwargs):
             return self
         if args[0].endswith("pools") and self._has_offset:
             if kwargs["params"]["offset"] != 50:
-                self.pool_response = {'body': []}
-                for x in range(50):
+                self.pool_response = {"body": []}
+                for _x in range(50):
                     self.pool_response["body"].append(
                         {
-                            'id': f'{"".join(random.sample(string.ascii_letters, 12))}',
-                            'subscriptionName': 'Red Hat Satellite Infrastructure Subscription',
-                            'entitlementsAvailable': random.randrange(100),
+                            "id": f'{"".join(random.sample(string.ascii_letters, 12))}',
+                            "subscriptionName": "Red Hat Satellite Infrastructure Subscription",
+                            "entitlementsAvailable": random.randrange(100),
                         }
                     )
                 return self
@@ -115,7 +116,7 @@ def get(self, *args, **kwargs):
             self.allocation_data = "this allocation data also includes entitlement data"
             return self
         if args[0].endswith("export"):
-            self.body = {'exportJobID': '123456', 'href': 'exportJob'}
+            self.body = {"exportJobID": "123456", "href": "exportJob"}
             return self
         if "exportJob" in args[0]:
             del self.status_code
@@ -123,7 +124,7 @@ def get(self, *args, **kwargs):
                 self._good_codes = [202]
             else:
                 self._good_codes = [202, 200]
-            self.body = {'exportID': 27, 'href': 'https://example.com/export/98ef892ac11'}
+            self.body = {"exportID": 27, "href": "https://example.com/export/98ef892ac11"}
             return self
         if "export" in args[0] and not args[0].endswith("export"):
             del self.status_code
@@ -133,8 +134,7 @@ def get(self, *args, **kwargs):
             return self
 
     def delete(self, *args, **kwargs):
-        """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester"""
-
+        """Simulate responses to DELETE requests for RHSM API endpoints used by Manifester."""
         if args[0].endswith("allocations/1234567890") and kwargs["params"]["force"] == "true":
             del self.status_code
             self.content = b""
@@ -143,9 +143,7 @@ def delete(self, *args, **kwargs):
 
 
 def test_basic_init(manifest_category="golden_ticket"):
-    """Test that manifester can initialize with the minimum required arguments and verify that
-    resulting object has an access token attribute"""
-
+    """Test that manifester can initialize with the minimum required arguments."""
     manifester_inst = Manifester(
         manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None)
     )
@@ -154,16 +152,14 @@ def test_basic_init(manifest_category="golden_ticket"):
 
 
 def test_create_allocation():
-    """Test that manifester's create_subscription_allocation method returns a UUID"""
-
+    """Test that manifester's create_subscription_allocation method returns a UUID."""
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     allocation_uuid = manifester.create_subscription_allocation()
     assert allocation_uuid.uuid == "1234567890"
 
 
 def test_negative_simple_retry_timeout():
-    """Test that exceeding the attempt limit when retrying a failed API call results in an exception"""
-
+    """Test that exceeding the attempt limit when retrying a failed API call results in an exception."""
     manifester = Manifester(
         manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=0)
     )
@@ -174,8 +170,7 @@ def test_negative_simple_retry_timeout():
 
 
 def test_negative_manifest_export_timeout():
-    """Test that exceeding the attempt limit when exporting a manifest results in an exception"""
-
+    """Test that exceeding the attempt limit when exporting a manifest results in an exception."""
     with pytest.raises(Exception) as exception:
         Manifester(
             manifest_category="golden_ticket",
@@ -185,8 +180,7 @@ def test_negative_manifest_export_timeout():
 
 
 def test_get_manifest():
-    """Test that manifester's get_manifest method returns a manifest"""
-
+    """Test that manifester's get_manifest method returns a manifest."""
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifest = manifester.get_manifest()
     assert manifest.content.decode("utf-8") == "this is a simulated manifest"
@@ -194,8 +188,7 @@ def test_get_manifest():
 
 
 def test_delete_subscription_allocation():
-    """Test that manifester's delete_subscription_allocation method deletes a subscription allocation"""
-
+    """Test that manifester's delete_subscription_allocation method deletes a subscription allocation."""
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
     response = manifester.delete_subscription_allocation()
@@ -204,8 +197,7 @@ def test_delete_subscription_allocation():
 
 
 def test_ingest_manifest_data_via_dict():
-    """Test that manifester is able to read configuration data from a dictionary"""
-
+    """Test that manifester is able to read configuration data from a dictionary."""
     manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     assert manifester.subscription_data == manifest_data["subscription_data"]
     assert manifester.simple_content_access == manifest_data["simple_content_access"]
@@ -215,10 +207,7 @@ def test_ingest_manifest_data_via_dict():
 
 
 def test_get_subscription_pools_with_offset():
-    """Tests that manifester can retrieve all subscription pools from an account containing
-    more than 50 pools
-    """
-
+    """Tests that manifester can retrieve all pools from an account containing more than 50 pools."""
     manifester = Manifester(
         manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True)
     )
@@ -227,10 +216,7 @@ def test_get_subscription_pools_with_offset():
 
 
 def test_correct_subs_added_to_allocation():
-    """Test that the subscriptions added to the subscription allocation match the subscription data
-    defined in manifester's config
-    """
-
+    """Test that subs added to the allocation match the subscription data in manifester's config."""
     manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
     active_subs = sorted([x["subscriptionName"] for x in manifester._active_pools])
@@ -239,8 +225,7 @@ def test_correct_subs_added_to_allocation():
 
 
 def test_invalid_sat_version():
-    """Test that an invalid sat_version value will be replaced with the latest valid sat_version"""
-
+    """Test that an invalid sat_version value will be replaced with the latest valid sat_version."""
     manifest_data["sat_version"] = "sat-6.20"
     manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     assert manifester.sat_version == "sat-6.14"

From 6e1a3d200e785e4a021374728590900775508b5c Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Mon, 4 Dec 2023 14:46:26 -0500
Subject: [PATCH 21/26] Modify workflow to workaround CodeQL failure

---
 .github/workflows/codeql-analysis.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
index 609cdea..e95ef74 100644
--- a/.github/workflows/codeql-analysis.yml
+++ b/.github/workflows/codeql-analysis.yml
@@ -42,5 +42,4 @@ jobs:
         pip install -U pip
         pip install -U .[test]
         cp manifester_settings.yaml.example manifester_settings.yaml
-        manifester --help
         pytest -v tests/

From 79a4b4ec97343f3d350dd62207e328172a8361fd Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Mon, 4 Dec 2023 14:51:56 -0500
Subject: [PATCH 22/26] Add pytest dep and correct typo in test manifest_data

---
 pyproject.toml           | 1 +
 tests/test_manifester.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/pyproject.toml b/pyproject.toml
index b5218d9..696d557 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -41,6 +41,7 @@ dependencies = [
     "click",
     "dynaconf",
     "logzero",
+    "pytest",
     "pyyaml",
     "requests",
     "setuptools",
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 00e453e..2f1bbcc 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -11,7 +11,7 @@
 manifest_data = {
     "log_level": "debug",
     "offline_token": "test",
-    "proxies": {"https:"},
+    "proxies": {"https": ""},
     "url": {
         "token_request": "https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token",
         "allocations": "https://api.access.redhat.com/management/v1/allocations",

From 2af7760e7afca3f9a79f024d20ea5c81b1c5f3d7 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Mon, 4 Dec 2023 16:02:14 -0500
Subject: [PATCH 23/26] Use manifest_data dict for all tests

Since the manifester repo does not contain a fully populated config
file, this commit switches all of the tests to use the manifest_data
dictionary defined in the test_manifester.py module as the value of
manifest_category when instantiating a Manifester object. It also add
Python 3.12 to the list of classifiers in pyproject.toml.
---
 pyproject.toml           |  1 +
 tests/test_manifester.py | 26 +++++++++++++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 696d557..6636938 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -36,6 +36,7 @@ classifiers = [
     "Programming Language :: Python :: 3",
     "Programming Language :: Python :: 3.10",
     "Programming Language :: Python :: 3.11",
+    "Programming Language :: Python :: 3.12",
 ]
 dependencies = [
     "click",
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 2f1bbcc..33bafdd 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -26,6 +26,14 @@
             "name": "Red Hat Satellite Infrastructure Subscription",
             "quantity": 1,
         },
+        {
+            "name": "Red Hat Beta Access",
+            "quantity": 1,
+        },
+        {
+            "name": "Red Hat Enterprise Linux for Virtual Datacenters, Premium",
+            "quantity": 1,
+        },
     ],
     "simple_content_access": "enabled",
 }
@@ -142,10 +150,10 @@ def delete(self, *args, **kwargs):
             return self
 
 
-def test_basic_init(manifest_category="golden_ticket"):
+def test_basic_init():
     """Test that manifester can initialize with the minimum required arguments."""
     manifester_inst = Manifester(
-        manifest_category=manifest_category, requester=RhsmApiStub(in_dict=None)
+        manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None)
     )
     assert isinstance(manifester_inst, Manifester)
     assert manifester_inst.access_token == "this is a simulated access token"
@@ -153,7 +161,7 @@ def test_basic_init(manifest_category="golden_ticket"):
 
 def test_create_allocation():
     """Test that manifester's create_subscription_allocation method returns a UUID."""
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     allocation_uuid = manifester.create_subscription_allocation()
     assert allocation_uuid.uuid == "1234567890"
 
@@ -161,7 +169,7 @@ def test_create_allocation():
 def test_negative_simple_retry_timeout():
     """Test that exceeding the attempt limit when retrying a failed API call results in an exception."""
     manifester = Manifester(
-        manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, fail_rate=0)
+        manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None, fail_rate=0)
     )
     manifester.requester._fail_rate = 100
     with pytest.raises(Exception) as exception:
@@ -173,7 +181,7 @@ def test_negative_manifest_export_timeout():
     """Test that exceeding the attempt limit when exporting a manifest results in an exception."""
     with pytest.raises(Exception) as exception:
         Manifester(
-            manifest_category="golden_ticket",
+            manifest_category=manifest_data,
             requester=RhsmApiStub(in_dict={"force_export_failure": True}),
         )
     assert str(exception.value) == "Export timeout exceeded"
@@ -181,7 +189,7 @@ def test_negative_manifest_export_timeout():
 
 def test_get_manifest():
     """Test that manifester's get_manifest method returns a manifest."""
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     manifest = manifester.get_manifest()
     assert manifest.content.decode("utf-8") == "this is a simulated manifest"
     assert manifest.status_code == 200
@@ -189,7 +197,7 @@ def test_get_manifest():
 
 def test_delete_subscription_allocation():
     """Test that manifester's delete_subscription_allocation method deletes a subscription allocation."""
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
     response = manifester.delete_subscription_allocation()
     assert response.status_code == 204
@@ -209,7 +217,7 @@ def test_ingest_manifest_data_via_dict():
 def test_get_subscription_pools_with_offset():
     """Tests that manifester can retrieve all pools from an account containing more than 50 pools."""
     manifester = Manifester(
-        manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None, has_offset=True)
+        manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None, has_offset=True)
     )
     manifester.get_manifest()
     assert len(manifester.subscription_pools["body"]) > 50
@@ -217,7 +225,7 @@ def test_get_subscription_pools_with_offset():
 
 def test_correct_subs_added_to_allocation():
     """Test that subs added to the allocation match the subscription data in manifester's config."""
-    manifester = Manifester(manifest_category="golden_ticket", requester=RhsmApiStub(in_dict=None))
+    manifester = Manifester(manifest_category=manifest_data, requester=RhsmApiStub(in_dict=None))
     manifester.get_manifest()
     active_subs = sorted([x["subscriptionName"] for x in manifester._active_pools])
     sub_names_from_config = sorted([x["NAME"] for x in manifester.subscription_data])

From 2bc257230bf661d10188f24cf1a6282c5226e5bd Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 5 Dec 2023 14:44:36 -0500
Subject: [PATCH 24/26] Modify checkout action to pull latest commit from PR

---
 .github/workflows/codeql-analysis.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
index e95ef74..4813d29 100644
--- a/.github/workflows/codeql-analysis.yml
+++ b/.github/workflows/codeql-analysis.yml
@@ -23,6 +23,8 @@ jobs:
     steps:
     - name: Checkout repository
       uses: actions/checkout@v4
+      with:
+        ref: ${{ github.ref }}
 
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v2

From 39483fa6a5582a59069232dfd9896e67f4e28d90 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 12 Dec 2023 14:53:02 -0500
Subject: [PATCH 25/26] Address reviewer feedback; delete status code in one
 additional spot

---
 manifester/manifester.py |  4 ++--
 tests/test_manifester.py | 13 ++++++++-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/manifester/manifester.py b/manifester/manifester.py
index 132661a..941d914 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -152,7 +152,7 @@ def subscription_pools(self):
 
         Returns a list of dictionaries containing metadata from the pools.
         """
-        max_results_per_page = 50
+        MAX_RESULTS_PER_PAGE = 50
         if not self._subscription_pools:
             _offset = 0
             data = {
@@ -252,7 +252,7 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
         Identifies pools that match the subscription names and quantities defined in settings, then
         attempts to add the specified quantity of each subscription to the allocation.
         """
-        success_code = 200
+        SUCCESS_CODE = 200
         logger.debug(f"Finding a matching pool for {subscription_data['name']}.")
         matching = [
             d
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 33bafdd..9df9c98 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -1,4 +1,5 @@
 from functools import cached_property
+from requests.exceptions import Timeout
 import random
 import string
 import uuid
@@ -94,6 +95,7 @@ def post(self, *args, **kwargs):
     def get(self, *args, **kwargs):
         """Simulate responses to GET requests for RHSM API endpoints used by Manifester."""
         if args[0].endswith("versions"):
+            del self.status_code
             self.version_response = {
                 "body": [
                     {"value": "sat-6.14", "description": "Satellite 6.14"},
@@ -179,11 +181,12 @@ def test_negative_simple_retry_timeout():
 
 def test_negative_manifest_export_timeout():
     """Test that exceeding the attempt limit when exporting a manifest results in an exception."""
-    with pytest.raises(Exception) as exception:
-        Manifester(
-            manifest_category=manifest_data,
-            requester=RhsmApiStub(in_dict={"force_export_failure": True}),
-        )
+    manifester = Manifester(
+        manifest_category=manifest_data,
+        requester=RhsmApiStub(in_dict={"force_export_failure": True}),
+    )
+    with pytest.raises(Timeout) as exception:
+        manifester.get_manifest()
     assert str(exception.value) == "Export timeout exceeded"
 
 

From 96e2640c63cc5935fd7a12640c34a3d7ff059143 Mon Sep 17 00:00:00 2001
From: synkd <dsynk@redhat.com>
Date: Tue, 12 Dec 2023 16:15:46 -0500
Subject: [PATCH 26/26] Address settings issue and test flakiness

This commit removes default values in several `get()` calls that were
pulling from settings and causing the unit tests to fail in CI. It also
changes the default `fail_rate` for HTTP response codes to 0 to address
an issue in which a bad response code would loop permanently, causing
unpredictable test flakiness.
---
 manifester/helpers.py    |  2 +-
 manifester/manifester.py | 38 +++++++++++++++++---------------------
 tests/test_manifester.py |  4 ++--
 3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/manifester/helpers.py b/manifester/helpers.py
index d202a1b..3be059c 100644
--- a/manifester/helpers.py
+++ b/manifester/helpers.py
@@ -48,7 +48,7 @@ def process_sat_version(sat_version, valid_sat_versions):
     return sat_version
 
 
-def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=20):
+def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=0):
     """Return an HTTP response code randomly selected from sets of good and bad codes."""
     if random.random() > (fail_rate / 100):
         return random.choice(good_codes)
diff --git a/manifester/manifester.py b/manifester/manifester.py
index 941d914..46f7293 100644
--- a/manifester/manifester.py
+++ b/manifester/manifester.py
@@ -44,12 +44,8 @@ def __init__(self, manifest_category, allocation_name=None, **kwargs):
         self.simple_content_access = kwargs.get(
             "simple_content_access", self.manifest_data.simple_content_access
         )
-        self.token_request_url = self.manifest_data.get("url", {}).get(
-            "token_request", settings.url.token_request
-        )
-        self.allocations_url = self.manifest_data.get("url", {}).get(
-            "allocations", settings.url.allocations
-        )
+        self.token_request_url = self.manifest_data.get("url").get("token_request")
+        self.allocations_url = self.manifest_data.get("url").get("allocations")
         self._access_token = None
         self._subscription_pools = None
         self._active_pools = []
@@ -83,7 +79,7 @@ def valid_sat_versions(self):
         """Retrieves the list of valid Satellite versions from the RHSM API."""
         headers = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
         }
         sat_versions_response = simple_retry(
             self.requester.get,
@@ -99,7 +95,7 @@ def create_subscription_allocation(self):
         """Creates a new consumer in the provided RHSM account and returns its UUID."""
         allocation_data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
             "params": {
                 "name": f"{self.allocation_name}",
                 "version": f"{self.sat_version}",
@@ -119,7 +115,7 @@ def create_subscription_allocation(self):
                 cmd_args=[f"{self.allocations_url}/{self.allocation_uuid}"],
                 cmd_kwargs={
                     "headers": {"Authorization": f"Bearer {self.access_token}"},
-                    "proxies": self.manifest_data.get("proxies", settings.proxies),
+                    "proxies": self.manifest_data.get("proxies"),
                     "json": {"simpleContentAccess": "disabled"},
                 },
             )
@@ -134,7 +130,7 @@ def delete_subscription_allocation(self):
         self._access_token = None
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
             "params": {"force": "true"},
         }
         if self.is_mock:
@@ -157,7 +153,7 @@ def subscription_pools(self):
             _offset = 0
             data = {
                 "headers": {"Authorization": f"Bearer {self.access_token}"},
-                "proxies": self.manifest_data.get("proxies", settings.proxies),
+                "proxies": self.manifest_data.get("proxies"),
                 "params": {"offset": _offset},
             }
             self._subscription_pools = simple_retry(
@@ -172,12 +168,12 @@ def subscription_pools(self):
             # For organizations with more than 50 subscription pools, the loop below works around
             # this limit by repeating calls with a progressively larger value for the `offset`
             # parameter.
-            while _results == max_results_per_page:
+            while _results == MAX_RESULTS_PER_PAGE:
                 _offset += 50
                 logger.debug(f"Fetching additional subscription pools with an offset of {_offset}.")
                 data = {
                     "headers": {"Authorization": f"Bearer {self.access_token}"},
-                    "proxies": self.manifest_data.get("proxies", settings.proxies),
+                    "proxies": self.manifest_data.get("proxies"),
                     "params": {"offset": _offset},
                 }
                 offset_pools = simple_retry(
@@ -199,7 +195,7 @@ def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
         """Attempts to add the set of subscriptions defined in the settings to the allocation."""
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
             "params": {"pool": f"{pool_id}", "quantity": f"{entitlement_quantity}"},
         }
         add_entitlements = simple_retry(
@@ -214,7 +210,7 @@ def verify_allocation_entitlements(self, entitlement_quantity, subscription_name
         logger.info(f"Verifying the entitlement quantity of {subscription_name} on the allocation.")
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
             "params": {"include": "entitlements"},
         }
         self.entitlement_data = simple_retry(
@@ -312,7 +308,7 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
                         )
                         self._active_pools.append(match)
                         break
-                elif add_entitlements.status_code == success_code:
+                elif add_entitlements.status_code == SUCCESS_CODE:
                     logger.debug(
                         f"Successfully added {subscription_data['quantity']} entitlements of "
                         f"{subscription_data['name']} to the allocation."
@@ -331,11 +327,11 @@ def trigger_manifest_export(self):
         Starts the export job, monitors the status of the job, and downloads the manifest on
         successful completion of the job.
         """
-        max_requests = 50
-        success_code = 200
+        MAX_REQUESTS = 50
+        SUCCESS_CODE = 200
         data = {
             "headers": {"Authorization": f"Bearer {self.access_token}"},
-            "proxies": self.manifest_data.get("proxies", settings.proxies),
+            "proxies": self.manifest_data.get("proxies"),
         }
         local_file = Path(f"manifests/{self.manifest_name}")
         local_file.parent.mkdir(parents=True, exist_ok=True)
@@ -355,7 +351,7 @@ def trigger_manifest_export(self):
         )
         request_count = 1
         limit_exceeded = False
-        while export_job.status_code != success_code:
+        while export_job.status_code != SUCCESS_CODE:
             export_job = simple_retry(
                 self.requester.get,
                 cmd_args=[
@@ -364,7 +360,7 @@ def trigger_manifest_export(self):
                 cmd_kwargs=data,
             )
             logger.debug(f"Attempting to export manifest. Attempt number: {request_count}")
-            if request_count > max_requests:
+            if request_count > MAX_REQUESTS:
                 limit_exceeded = True
                 logger.info(
                     "Manifest export job status check limit exceeded. This may indicate an "
diff --git a/tests/test_manifester.py b/tests/test_manifester.py
index 9df9c98..b9145a8 100644
--- a/tests/test_manifester.py
+++ b/tests/test_manifester.py
@@ -1,10 +1,10 @@
 from functools import cached_property
-from requests.exceptions import Timeout
 import random
 import string
 import uuid
 
 import pytest
+from requests.exceptions import Timeout
 
 from manifester import Manifester
 from manifester.helpers import MockStub, fake_http_response_code
@@ -71,7 +71,7 @@ class RhsmApiStub(MockStub):
     def __init__(self, in_dict=None, **kwargs):
         self._good_codes = kwargs.get("good_codes", [200])
         self._bad_codes = kwargs.get("bad_codes", [429, 500, 504])
-        self._fail_rate = kwargs.get("fail_rate", 10)
+        self._fail_rate = kwargs.get("fail_rate", 0)
         self._has_offset = kwargs.get("has_offset", False)
         super().__init__(in_dict)