From 9420c1893d6c03d1b3d589788fbf5e15ea4ea3a2 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 11:59:06 -0500 Subject: [PATCH 1/9] feat: Add improved API client with better error handling and configuration --- src/game_sdk/game/api_client.py | 144 ++++++++++++++++++++++++++++++++ src/game_sdk/game/config.py | 23 +++++ src/game_sdk/game/exceptions.py | 26 ++++++ 3 files changed, 193 insertions(+) create mode 100644 src/game_sdk/game/api_client.py create mode 100644 src/game_sdk/game/config.py create mode 100644 src/game_sdk/game/exceptions.py diff --git a/src/game_sdk/game/api_client.py b/src/game_sdk/game/api_client.py new file mode 100644 index 00000000..fd7e1658 --- /dev/null +++ b/src/game_sdk/game/api_client.py @@ -0,0 +1,144 @@ +""" +API client module for the GAME SDK. + +This module provides a dedicated API client for making requests to the GAME API, +handling authentication, errors, and response parsing consistently. +""" + +import requests +from typing import Dict, Any, Optional +from game_sdk.game.config import config +from game_sdk.game.exceptions import APIError, AuthenticationError, ValidationError +from game_sdk.game.custom_types import ActionResponse, FunctionResult +from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type + + +class GameAPIClient: + """Client for interacting with the GAME API. + + This class handles all API communication, including authentication, + request retries, and error handling. + + Attributes: + api_key (str): API key for authentication + base_url (str): Base URL for API requests + session (requests.Session): Reusable session for API requests + """ + + def __init__(self, api_key: str): + """Initialize the API client. + + Args: + api_key (str): API key for authentication + + Raises: + ValueError: If API key is not provided + """ + if not api_key: + raise ValueError("API key is required") + + self.api_key = api_key + self.base_url = config.api_url + self.session = requests.Session() + self.session.headers.update({ + "Authorization": f"Bearer {api_key}", + "Content-Type": "application/json" + }) + + @retry( + stop=stop_after_attempt(3), + wait=wait_exponential(multiplier=1, min=4, max=10), + retry=retry_if_exception_type(APIError) + ) + def make_request( + self, + method: str, + endpoint: str, + data: Optional[Dict[str, Any]] = None, + params: Optional[Dict[str, Any]] = None + ) -> Dict[str, Any]: + """Make an HTTP request to the API. + + Args: + method (str): HTTP method (GET, POST, etc.) + endpoint (str): API endpoint + data (Optional[Dict[str, Any]], optional): Request body. Defaults to None. + params (Optional[Dict[str, Any]], optional): Query parameters. Defaults to None. + + Raises: + AuthenticationError: If authentication fails + ValidationError: If request validation fails + APIError: For other API-related errors + + Returns: + Dict[str, Any]: API response data + """ + url = f"{self.base_url}/{endpoint.lstrip('/')}" + + try: + response = self.session.request( + method=method, + url=url, + json=data, + params=params + ) + + response.raise_for_status() + return response.json() + + except requests.exceptions.HTTPError as e: + if response.status_code == 401: + raise AuthenticationError("Authentication failed") from e + elif response.status_code == 422: + raise ValidationError("Invalid request data") from e + else: + raise APIError(f"API request failed: {str(e)}") from e + except requests.exceptions.RequestException as e: + raise APIError(f"Request failed: {str(e)}") from e + + def get(self, endpoint: str, params: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """Make a GET request. + + Args: + endpoint (str): API endpoint + params (Optional[Dict[str, Any]], optional): Query parameters. Defaults to None. + + Returns: + Dict[str, Any]: API response data + """ + return self.make_request("GET", endpoint, params=params) + + def post(self, endpoint: str, data: Dict[str, Any]) -> Dict[str, Any]: + """Make a POST request. + + Args: + endpoint (str): API endpoint + data (Dict[str, Any]): Request body + + Returns: + Dict[str, Any]: API response data + """ + return self.make_request("POST", endpoint, data=data) + + def put(self, endpoint: str, data: Dict[str, Any]) -> Dict[str, Any]: + """Make a PUT request. + + Args: + endpoint (str): API endpoint + data (Dict[str, Any]): Request body + + Returns: + Dict[str, Any]: API response data + """ + return self.make_request("PUT", endpoint, data=data) + + def delete(self, endpoint: str) -> Dict[str, Any]: + """Make a DELETE request. + + Args: + endpoint (str): API endpoint + + Returns: + Dict[str, Any]: API response data + """ + return self.make_request("DELETE", endpoint) diff --git a/src/game_sdk/game/config.py b/src/game_sdk/game/config.py new file mode 100644 index 00000000..ad57439b --- /dev/null +++ b/src/game_sdk/game/config.py @@ -0,0 +1,23 @@ +""" +Configuration module for the GAME SDK. + +This module provides centralized configuration management for the SDK. +""" + +from dataclasses import dataclass + + +@dataclass +class Config: + """Configuration settings for the GAME SDK. + + Attributes: + api_url (str): Base URL for API requests + default_timeout (int): Default timeout for API requests in seconds + """ + api_url: str = "https://sdk.game.virtuals.io" + default_timeout: int = 30 + + +# Global configuration instance +config = Config() diff --git a/src/game_sdk/game/exceptions.py b/src/game_sdk/game/exceptions.py new file mode 100644 index 00000000..3078a59d --- /dev/null +++ b/src/game_sdk/game/exceptions.py @@ -0,0 +1,26 @@ +""" +Custom exceptions for the GAME SDK. + +This module provides custom exception classes for better error handling +and more informative error messages. +""" + + +class GameSDKError(Exception): + """Base exception class for all GAME SDK errors.""" + pass + + +class APIError(GameSDKError): + """Raised when an API request fails.""" + pass + + +class AuthenticationError(APIError): + """Raised when API authentication fails.""" + pass + + +class ValidationError(APIError): + """Raised when request validation fails.""" + pass From 4705c4e051ad80b43d5342b2403fa595443ccd17 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:01:21 -0500 Subject: [PATCH 2/9] test: Add comprehensive tests for API client --- requirements-dev.txt | 3 + tests/test_api_client.py | 214 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+) create mode 100644 requirements-dev.txt create mode 100644 tests/test_api_client.py diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 00000000..b3f34bcb --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,3 @@ +pytest>=7.0.0 +responses>=0.23.0 +pytest-cov>=4.0.0 diff --git a/tests/test_api_client.py b/tests/test_api_client.py new file mode 100644 index 00000000..9cb2ebf2 --- /dev/null +++ b/tests/test_api_client.py @@ -0,0 +1,214 @@ +""" +Tests for the GAME SDK API client. + +This module contains tests for the GameAPIClient class, including error handling, +retry logic, and HTTP method wrappers. +""" + +import pytest +import responses +from requests.exceptions import HTTPError, RequestException +from game_sdk.game.api_client import GameAPIClient +from game_sdk.game.exceptions import APIError, AuthenticationError, ValidationError +from game_sdk.game.config import config + + +@pytest.fixture +def api_client(): + """Create a test API client instance.""" + return GameAPIClient("test_api_key") + + +@pytest.fixture +def mock_responses(): + """Set up mock responses for testing.""" + with responses.RequestsMock() as rsps: + yield rsps + + +def test_init_with_valid_api_key(api_client): + """Test client initialization with valid API key.""" + assert api_client.api_key == "test_api_key" + assert api_client.base_url == config.api_url + assert api_client.session.headers["Authorization"] == "Bearer test_api_key" + assert api_client.session.headers["Content-Type"] == "application/json" + + +def test_init_without_api_key(): + """Test client initialization without API key raises error.""" + with pytest.raises(ValueError, match="API key is required"): + GameAPIClient("") + + +def test_get_request_success(api_client, mock_responses): + """Test successful GET request.""" + expected_response = {"data": "test_data"} + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + response = api_client.get("test") + assert response == expected_response + + +def test_post_request_success(api_client, mock_responses): + """Test successful POST request.""" + request_data = {"key": "value"} + expected_response = {"data": "created"} + mock_responses.add( + responses.POST, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + response = api_client.post("test", request_data) + assert response == expected_response + + +def test_put_request_success(api_client, mock_responses): + """Test successful PUT request.""" + request_data = {"key": "updated_value"} + expected_response = {"data": "updated"} + mock_responses.add( + responses.PUT, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + response = api_client.put("test", request_data) + assert response == expected_response + + +def test_delete_request_success(api_client, mock_responses): + """Test successful DELETE request.""" + expected_response = {"data": "deleted"} + mock_responses.add( + responses.DELETE, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + response = api_client.delete("test") + assert response == expected_response + + +def test_authentication_error(api_client, mock_responses): + """Test authentication error handling.""" + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json={"error": "Unauthorized"}, + status=401 + ) + + with pytest.raises(AuthenticationError, match="Authentication failed"): + api_client.get("test") + + +def test_validation_error(api_client, mock_responses): + """Test validation error handling.""" + mock_responses.add( + responses.POST, + f"{config.api_url}/test", + json={"error": "Invalid data"}, + status=422 + ) + + with pytest.raises(ValidationError, match="Invalid request data"): + api_client.post("test", {}) + + +def test_api_error(api_client, mock_responses): + """Test general API error handling.""" + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json={"error": "Server error"}, + status=500 + ) + + with pytest.raises(APIError, match="API request failed"): + api_client.get("test") + + +def test_network_error(api_client, mock_responses): + """Test network error handling.""" + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + body=RequestException("Network error") + ) + + with pytest.raises(APIError, match="Request failed"): + api_client.get("test") + + +@pytest.mark.parametrize("status_code", [500, 502, 503, 504]) +def test_retry_on_server_error(api_client, mock_responses, status_code): + """Test retry logic on server errors.""" + # First two requests fail, third succeeds + expected_response = {"data": "success"} + + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json={"error": "Server error"}, + status=status_code + ) + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json={"error": "Server error"}, + status=status_code + ) + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + response = api_client.get("test") + assert response == expected_response + assert len(mock_responses.calls) == 3 # Verify retry happened + + +def test_request_with_params(api_client, mock_responses): + """Test request with query parameters.""" + expected_response = {"data": "filtered"} + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + + params = {"filter": "value"} + response = api_client.get("test", params=params) + assert response == expected_response + assert "filter=value" in mock_responses.calls[0].request.url + + +def test_endpoint_path_handling(api_client, mock_responses): + """Test proper handling of endpoint paths with/without leading slash.""" + expected_response = {"data": "test"} + + # Test with leading slash + mock_responses.add( + responses.GET, + f"{config.api_url}/test", + json=expected_response, + status=200 + ) + response = api_client.get("/test") + assert response == expected_response + + # Test without leading slash + response = api_client.get("test") + assert response == expected_response From 591b060b7917ca3a81feb2c50511101a83442910 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:05:39 -0500 Subject: [PATCH 3/9] fix: Update API client error handling and retry logic --- src/game_sdk/game/api_client.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/game_sdk/game/api_client.py b/src/game_sdk/game/api_client.py index fd7e1658..10415560 100644 --- a/src/game_sdk/game/api_client.py +++ b/src/game_sdk/game/api_client.py @@ -48,7 +48,7 @@ def __init__(self, api_key: str): @retry( stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10), - retry=retry_if_exception_type(APIError) + retry=retry_if_exception_type((APIError, requests.exceptions.RequestException)) ) def make_request( self, @@ -74,7 +74,7 @@ def make_request( Dict[str, Any]: API response data """ url = f"{self.base_url}/{endpoint.lstrip('/')}" - + try: response = self.session.request( method=method, @@ -82,18 +82,22 @@ def make_request( json=data, params=params ) - + response.raise_for_status() return response.json() - + except requests.exceptions.HTTPError as e: if response.status_code == 401: + # Don't retry auth errors raise AuthenticationError("Authentication failed") from e elif response.status_code == 422: + # Don't retry validation errors raise ValidationError("Invalid request data") from e else: + # Retry other HTTP errors raise APIError(f"API request failed: {str(e)}") from e except requests.exceptions.RequestException as e: + # Retry network errors raise APIError(f"Request failed: {str(e)}") from e def get(self, endpoint: str, params: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: From c0399e17dfd29ac117feaa297a75c6bd51486b89 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:07:31 -0500 Subject: [PATCH 4/9] fix: Update retry logic to exclude auth and validation errors --- src/game_sdk/game/api_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/game_sdk/game/api_client.py b/src/game_sdk/game/api_client.py index 10415560..b9b96d7b 100644 --- a/src/game_sdk/game/api_client.py +++ b/src/game_sdk/game/api_client.py @@ -10,7 +10,7 @@ from game_sdk.game.config import config from game_sdk.game.exceptions import APIError, AuthenticationError, ValidationError from game_sdk.game.custom_types import ActionResponse, FunctionResult -from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception_type +from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception class GameAPIClient: @@ -45,10 +45,16 @@ def __init__(self, api_key: str): "Content-Type": "application/json" }) + def should_retry(self, exception): + """Determine if we should retry the request based on the exception type.""" + if isinstance(exception, (AuthenticationError, ValidationError)): + return False + return isinstance(exception, (APIError, requests.exceptions.RequestException)) + @retry( stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10), - retry=retry_if_exception_type((APIError, requests.exceptions.RequestException)) + retry=retry_if_exception(self.should_retry) ) def make_request( self, From f68f8650829acd5ee74404f8e8cbd5c99baa2227 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:08:03 -0500 Subject: [PATCH 5/9] fix: Make retry logic a static method --- src/game_sdk/game/api_client.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/game_sdk/game/api_client.py b/src/game_sdk/game/api_client.py index b9b96d7b..d51903f7 100644 --- a/src/game_sdk/game/api_client.py +++ b/src/game_sdk/game/api_client.py @@ -45,7 +45,8 @@ def __init__(self, api_key: str): "Content-Type": "application/json" }) - def should_retry(self, exception): + @staticmethod + def should_retry(exception): """Determine if we should retry the request based on the exception type.""" if isinstance(exception, (AuthenticationError, ValidationError)): return False @@ -54,7 +55,7 @@ def should_retry(self, exception): @retry( stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10), - retry=retry_if_exception(self.should_retry) + retry=retry_if_exception(should_retry) ) def make_request( self, From df79b4befaa56fa401d5ae821ac8d270335e837b Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:08:33 -0500 Subject: [PATCH 6/9] fix: Move retry predicate function outside class --- src/game_sdk/game/api_client.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/game_sdk/game/api_client.py b/src/game_sdk/game/api_client.py index d51903f7..5fc710f8 100644 --- a/src/game_sdk/game/api_client.py +++ b/src/game_sdk/game/api_client.py @@ -13,6 +13,13 @@ from tenacity import retry, stop_after_attempt, wait_exponential, retry_if_exception +def should_retry(exception): + """Determine if we should retry the request based on the exception type.""" + if isinstance(exception, (AuthenticationError, ValidationError)): + return False + return isinstance(exception, (APIError, requests.exceptions.RequestException)) + + class GameAPIClient: """Client for interacting with the GAME API. @@ -25,7 +32,7 @@ class GameAPIClient: session (requests.Session): Reusable session for API requests """ - def __init__(self, api_key: str): + def __init__(self, api_key: Optional[str] = None): """Initialize the API client. Args: @@ -45,13 +52,6 @@ def __init__(self, api_key: str): "Content-Type": "application/json" }) - @staticmethod - def should_retry(exception): - """Determine if we should retry the request based on the exception type.""" - if isinstance(exception, (AuthenticationError, ValidationError)): - return False - return isinstance(exception, (APIError, requests.exceptions.RequestException)) - @retry( stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=4, max=10), From fa6e5d2c934f63dfcff9c2743a0b848fae790745 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:09:53 -0500 Subject: [PATCH 7/9] test: Update test expectations for API and network errors --- tests/test_api_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 9cb2ebf2..865df5c6 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -133,7 +133,7 @@ def test_api_error(api_client, mock_responses): status=500 ) - with pytest.raises(APIError, match="API request failed"): + with pytest.raises(tenacity.RetryError): api_client.get("test") @@ -145,7 +145,7 @@ def test_network_error(api_client, mock_responses): body=RequestException("Network error") ) - with pytest.raises(APIError, match="Request failed"): + with pytest.raises(tenacity.RetryError): api_client.get("test") From d740caf862c9a053206f5232551394554963cf5c Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:11:05 -0500 Subject: [PATCH 8/9] test: Add tenacity import and update test assertions --- tests/test_api_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 865df5c6..4f108583 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -7,10 +7,11 @@ import pytest import responses +import tenacity from requests.exceptions import HTTPError, RequestException from game_sdk.game.api_client import GameAPIClient -from game_sdk.game.exceptions import APIError, AuthenticationError, ValidationError from game_sdk.game.config import config +from game_sdk.game.exceptions import APIError, AuthenticationError, ValidationError @pytest.fixture From 713b503621c987bccbed60abbc44a7280e09daf8 Mon Sep 17 00:00:00 2001 From: Dylan Burkey Date: Mon, 27 Jan 2025 12:14:04 -0500 Subject: [PATCH 9/9] chore: Add .coverage to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ac56288d..fcdf18ec 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ *.pyc *__pycache__ *.json +.coverage *.DS_Store dist/ \ No newline at end of file