VED-79: code review - timeouts, error class

Author: Akol125

mns_subscription/models/errors.py

@@ -78,7 +78,7 @@ def to_operation_outcome() -> dict:
 
 
 @dataclass
-class ResourceFoundError(RuntimeError):
+class ResourceNotFoundError(RuntimeError):
     """Return this error when the requested resource does not exist or not complete"""
 
     response: None

mns_subscription/src/mns_service.py

@@ -6,7 +6,7 @@
 from authentication import AppRestrictedAuth
 from models.errors import (
     UnhandledResponseError,
-    ResourceFoundError,
+    ResourceNotFoundError,
     UnauthorizedError,
     ServerError,
     BadRequestError,
@@ -46,14 +46,14 @@ def __init__(self, authenticator: AppRestrictedAuth):
 
     def subscribe_notification(self) -> dict | None:
 
-        response = requests.post(MNS_URL, headers=self.request_headers, data=json.dumps(self.subscription_payload))
+        response = requests.post(MNS_URL, headers=self.request_headers, data=json.dumps(self.subscription_payload),timeout=15)
         if response.status_code in (200, 201):
             return response.json()
         else:
             MnsService.handle_response(response)
 
     def get_subscription(self) -> dict | None:
-        response = requests.get(MNS_URL, headers=self.request_headers)
+        response = requests.get(MNS_URL, headers=self.request_headers, timeout=10)
         logging.info(f"GET {MNS_URL}")
         logging.debug(f"Headers: {self.request_headers}")
 
@@ -91,7 +91,7 @@ def check_subscription(self) -> dict:
     def delete_subscription(self, subscription_id: str) -> bool:
         """Delete the subscription by ID."""
         url = f"{MNS_URL}/{subscription_id}"
-        response = requests.delete(url, headers=self.request_headers)
+        response = requests.delete(url, headers=self.request_headers, timeout=10)
         if response.status_code in (200, 204):
             logging.info(f"Deleted subscription {subscription_id}")
             return True
@@ -120,7 +120,7 @@ def handle_response(response):
             400: (BadRequestError, "Bad request: Resource type or parameters incorrect"),
             403: (UnauthorizedError, "You don't have the right permissions for this request"),
             500: (ServerError, "Internal Server Error"),
-            404: (ResourceFoundError, "Subscription or Resource not found"),
+            404: (ResourceNotFoundError, "Subscription or Resource not found"),
             409: (ConflictError, "SQS Queue Already Subscribed, can't re-subscribe")
         }
         exception_class, error_message = error_mapping.get(

mns_subscription/tests/test_mns_service.py

@@ -9,7 +9,7 @@
     TokenValidationError,
     BadRequestError,
     UnauthorizedError,
-    ResourceFoundError)
+    ResourceNotFoundError)
 
 
 SQS_ARN = "arn:aws:sqs:eu-west-2:123456789012:my-queue"
@@ -60,7 +60,7 @@ def test_not_found_subscription(self, mock_post):
 
         service = MnsService(self.authenticator)
 
-        with self.assertRaises(ResourceFoundError) as context:
+        with self.assertRaises(ResourceNotFoundError) as context:
             service.subscribe_notification()
         self.assertIn("Subscription or Resource not found", str(context.exception))
 
@@ -191,7 +191,7 @@ def test_delete_subscription_404(self, mock_delete):
         mock_delete.return_value = mock_response
 
         service = MnsService(self.authenticator)
-        with self.assertRaises(ResourceFoundError):
+        with self.assertRaises(ResourceNotFoundError):
             service.delete_subscription("sub-id-123")
 
     @patch("mns_service.requests.delete")
@@ -263,7 +263,7 @@ def mock_response(self, status_code, json_data=None):
 
     def test_404_resource_found_error(self):
         resp = self.mock_response(404, {"resource": "Not found"})
-        with self.assertRaises(ResourceFoundError) as context:
+        with self.assertRaises(ResourceNotFoundError) as context:
             MnsService.handle_response(resp)
         self.assertIn("Subscription or Resource not found", str(context.exception))
         self.assertEqual(context.exception.message, "Subscription or Resource not found")