Merge pull request #218 from canonical/wd-2024

feat: Add better error handling for missing api key and username

Author: petesfrench

CHANGELOG.md

@@ -1,3 +1,7 @@
+### 7.1.1 [01-07-2025]
+**Added**
+- Better error handling when api key and username is missing
+
 ### 7.1.0 [17-11-2025]
 **Updated** EngagePages class
 - Support optional `value` in `get_engage_pages_tags` to return tags for a specific engage page type

canonicalwebteam/discourse/models.py

@@ -27,13 +27,27 @@ def __init__(
         self.base_url = base_url.rstrip("/")
         self.session = session
         self.get_topics_query_id = get_topics_query_id
+        self.api_key = api_key
+        self.api_username = api_username
 
         if api_key and api_username:
             self.session.headers = {
                 "Api-Key": api_key,
                 "Api-Username": api_username,
             }
 
+    def _require_authentication(self):
+        """
+        Check if API credentials are available and raise an error if not.
+        This should be called before accessing authenticated endpoints.
+        a.k.a. Data Explorer endpoints.
+        """
+        if not self.api_key or not self.api_username:
+            raise ValueError(
+                "API authentication required: API key and username "
+                "are required to access this endpoint"
+            )
+
     def __del__(self):
         self.session.close()
 
@@ -54,6 +68,8 @@ def get_topics(self, topic_ids):
         we are using it to obtain multiple Tutorials content without
         doing multiple API calls.
         """
+        self._require_authentication()
+
         headers = {
             "Accept": "application/json",
             "Content-Type": "multipart/form-data;",
@@ -167,6 +183,8 @@ def get_topic_list_by_category(self, category_id, limit=100, offset=0):
         - limit [int]: 100 by default, also set in data explorer
         - offset [int]: 0 by default (first page)
         """
+        self._require_authentication()
+
         # See https://discourse.ubuntu.com/admin/plugins/explorer?id=89
         data_explorer_id = 89
         headers = {
@@ -203,6 +221,8 @@ def get_topics_last_activity_time(self, topic_id):
         Args:
         - topic_id [int]: The topic ID
         """
+        self._require_authentication()
+
         # See https://discourse.ubuntu.com/admin/plugins/explorer?id=122
         data_explorer_id = 122
         headers = {
@@ -228,6 +248,8 @@ def get_categories_last_activity_time(self, category_id):
         Args:
         - category_id [int]: The category ID
         """
+        self._require_authentication()
+
         # See https://discourse.ubuntu.com/admin/plugins/explorer?id=123
         data_explorer_id = 123
         headers = {
@@ -331,6 +353,8 @@ def get_engage_pages_by_param(
         - limit [int]: 50 by default, also set in data explorer
         - offset [int]: 0 by default (first page)
         """
+        self._require_authentication()
+
         headers = {
             "Accept": "application/json",
             "Content-Type": "multipart/form-data;",
@@ -392,6 +416,8 @@ def get_engage_pages_by_tag(self, category_id, tag, limit=50, offset=0):
         - limit [int]: 50 by default, also set in data explorer
         - offset [int]: 0 by default (first page)
         """
+        self._require_authentication()
+
         headers = {
             "Accept": "application/json",
             "Content-Type": "multipart/form-data;",

setup.py

@@ -4,7 +4,7 @@
 
 setup(
     name="canonicalwebteam.discourse",
-    version="7.1.0",
+    version="7.1.1",
     author="Canonical webteam",
     author_email="webteam@canonical.com",
     url="https://github.com/canonical/canonicalwebteam.discourse",

tests/test_engage_pages.py

@@ -28,6 +28,8 @@ def setUp(self):
         self.discourse_api = DiscourseAPI(
             base_url="https://discourse.ubuntu.com/",
             session=session,
+            api_key="fake-api-key",
+            api_username="fake-username",
         )
         self.engage_pages = EngagePages(
             category_id=51,

tests/test_models.py

@@ -26,6 +26,11 @@ def test_get_topic(self):
         self.assertEqual(topic["id"], 34)
         self.assertEqual(topic["title"], "An index page")
 
+    def test_require_authentication_raises_error_without_credentials(self):
+        with self.assertRaises(ValueError) as context:
+            self.api._require_authentication()
+        self.assertIn("API authentication required", str(context.exception))
+
 
 if __name__ == "__main__":
     unittest.main()