-
Notifications
You must be signed in to change notification settings - Fork 10
Enhance CoinGecko API Configuration and Error Handling Framework #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
80a67b8
8daa19b
dba80a5
330862c
268e7b3
1a13841
f9e351b
d464bc3
fab7046
657f18e
9d5aa36
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,48 +1,62 @@ | ||
| .venv | ||
| <<<<<<< HEAD | ||
| __pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| .pytest_cache/ | ||
| .env | ||
| __pycache__ | ||
| .pytest_cache | ||
| .pypirc | ||
| *.db | ||
| test | ||
| test_state.json | ||
| task_flow.egg-info | ||
| example_repo | ||
| signature.js | ||
| git-filter-repo | ||
| task/orca/ | ||
| **/dist/ | ||
| # yarn.lock | ||
| package-lock.json | ||
| node_modules | ||
| build | ||
| migrate.sh | ||
| */dev.js | ||
| executables/* | ||
| namespace/* | ||
| config/* | ||
| .env.local | ||
| taskStateInfoKeypair.json | ||
| localKOIIDB.db | ||
| metadata.json | ||
| .npmrc | ||
| *.pem | ||
| .vscode | ||
| .cursor | ||
| data/chunks | ||
| data/process | ||
| test_state.csv | ||
| todos-example.csv | ||
|
|
||
|
|
||
| # Ignore auto-generated repository directories | ||
| repos/ | ||
| .venv/ | ||
| venv/ | ||
| dist/ | ||
| build/ | ||
| *.egg-info/ | ||
| .DS_Store | ||
| ======= | ||
| # Python | ||
| __pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| *.so | ||
| .Python | ||
| build/ | ||
| develop-eggs/ | ||
| dist/ | ||
| downloads/ | ||
| eggs/ | ||
| .eggs/ | ||
| lib/ | ||
| lib64/ | ||
| parts/ | ||
| sdist/ | ||
| var/ | ||
| wheels/ | ||
| *.egg-info/ | ||
| .installed.cfg | ||
| *.egg | ||
|
|
||
| # Virtual environments | ||
| venv/ | ||
| env/ | ||
| .env | ||
| .venv | ||
|
|
||
| # Ignore Data | ||
| data/* | ||
| # Pytest | ||
| .pytest_cache/ | ||
|
|
||
| # IDEs and editors | ||
| .idea/ | ||
| .vscode/ | ||
| *.swp | ||
| *.swo | ||
|
|
||
| venv | ||
| # Logs | ||
| *.log | ||
|
|
||
| **/venv/ | ||
| # OS generated files | ||
| .DS_Store | ||
| .DS_Store? | ||
| ._* | ||
| .Spotlight-V100 | ||
| .Trashes | ||
| ehthumbs.db | ||
| Thumbs.db | ||
| >>>>>>> pr-24-HermanKoii-prometheus-test |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,122 @@ | ||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||
| from typing import Optional, Dict, Any | ||||||||||||||||||||||||||||||||||||||
| from dotenv import load_dotenv | ||||||||||||||||||||||||||||||||||||||
| import re | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| class InvalidConfigurationError(ValueError): | ||||||||||||||||||||||||||||||||||||||
| """Custom exception for invalid configuration.""" | ||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| class CoinGeckoConfig: | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Configuration management for CoinGecko API client. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Supports configuration through: | ||||||||||||||||||||||||||||||||||||||
| 1. Environment variables | ||||||||||||||||||||||||||||||||||||||
| 2. Explicit parameters | ||||||||||||||||||||||||||||||||||||||
| 3. Default values | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def __init__( | ||||||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||||||
| api_base_url: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||
| api_key: Optional[str] = None, | ||||||||||||||||||||||||||||||||||||||
| timeout: Optional[int] = None, | ||||||||||||||||||||||||||||||||||||||
| max_retries: Optional[int] = None | ||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Initialize CoinGecko API configuration. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||||||
| api_base_url (Optional[str]): Base URL for CoinGecko API | ||||||||||||||||||||||||||||||||||||||
| api_key (Optional[str]): API key for authentication | ||||||||||||||||||||||||||||||||||||||
| timeout (Optional[int]): Request timeout in seconds | ||||||||||||||||||||||||||||||||||||||
| max_retries (Optional[int]): Maximum number of request retries | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Raises: | ||||||||||||||||||||||||||||||||||||||
| InvalidConfigurationError: If configuration parameters are invalid | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| # Load .env file if it exists | ||||||||||||||||||||||||||||||||||||||
| load_dotenv() | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Priority: Explicit parameters > Environment Variables > Default Values | ||||||||||||||||||||||||||||||||||||||
| self.api_base_url = ( | ||||||||||||||||||||||||||||||||||||||
| api_base_url or | ||||||||||||||||||||||||||||||||||||||
| os.getenv('COINGECKO_API_BASE_URL', 'https://api.coingecko.com/api/v3') | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| self.api_key = ( | ||||||||||||||||||||||||||||||||||||||
| api_key or | ||||||||||||||||||||||||||||||||||||||
| os.getenv('COINGECKO_API_KEY') | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| self.timeout = ( | ||||||||||||||||||||||||||||||||||||||
| timeout or | ||||||||||||||||||||||||||||||||||||||
| int(os.getenv('COINGECKO_API_TIMEOUT', 10)) | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| self.max_retries = ( | ||||||||||||||||||||||||||||||||||||||
| max_retries or | ||||||||||||||||||||||||||||||||||||||
| int(os.getenv('COINGECKO_MAX_RETRIES', 3)) | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix type issues with os.getenv default values. The self.timeout = (
timeout or
- int(os.getenv('COINGECKO_API_TIMEOUT', 10))
+ int(os.getenv('COINGECKO_API_TIMEOUT', '10'))
)
self.max_retries = (
max_retries or
- int(os.getenv('COINGECKO_MAX_RETRIES', 3))
+ int(os.getenv('COINGECKO_MAX_RETRIES', '3'))
)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[convention] 54-54: Trailing whitespace (C0303) [convention] 57-57: Trailing whitespace (C0303) [convention] 59-59: Trailing whitespace (C0303) [warning] 55-55: os.getenv default type is builtins.int. Expected str or None. (W1508) [warning] 60-60: os.getenv default type is builtins.int. Expected str or None. (W1508) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Validate configuration during initialization | ||||||||||||||||||||||||||||||||||||||
| self._validate() | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def get_config(self) -> Dict[str, Any]: | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Get current configuration as a dictionary. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||
| Dict[str, Any]: Configuration dictionary | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||
| 'api_base_url': self.api_base_url, | ||||||||||||||||||||||||||||||||||||||
| 'api_key': '****' if self.api_key else None, # Mask API key | ||||||||||||||||||||||||||||||||||||||
| 'timeout': self.timeout, | ||||||||||||||||||||||||||||||||||||||
| 'max_retries': self.max_retries | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def _validate(self) -> None: | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Validate the current configuration. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Raises: | ||||||||||||||||||||||||||||||||||||||
| InvalidConfigurationError: If configuration is invalid | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| # Validate API base URL | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(self.api_base_url, str): | ||||||||||||||||||||||||||||||||||||||
| raise InvalidConfigurationError("API base URL must be a string") | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Use regex to validate URL structure | ||||||||||||||||||||||||||||||||||||||
| url_pattern = re.compile( | ||||||||||||||||||||||||||||||||||||||
| r'^https?://' # http:// or https:// | ||||||||||||||||||||||||||||||||||||||
| r'(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\\.)+(?:[A-Z]{2,6}\\.?|[A-Z0-9-]{2,}\\.?)|' # domain | ||||||||||||||||||||||||||||||||||||||
| r'localhost|' # localhost | ||||||||||||||||||||||||||||||||||||||
| r'\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})' # or IP | ||||||||||||||||||||||||||||||||||||||
| r'(?::\d+)?' # optional port | ||||||||||||||||||||||||||||||||||||||
| r'(?:/?|[/?]\S+)$', re.IGNORECASE) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if not url_pattern.match(self.api_base_url): | ||||||||||||||||||||||||||||||||||||||
| raise InvalidConfigurationError("Invalid API base URL format") | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Validate timeout | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(self.timeout, int) or self.timeout <= 0: | ||||||||||||||||||||||||||||||||||||||
| raise InvalidConfigurationError("Timeout must be a positive integer") | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| # Validate max retries | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(self.max_retries, int) or self.max_retries < 0: | ||||||||||||||||||||||||||||||||||||||
| raise InvalidConfigurationError("Max retries must be a non-negative integer") | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def validate(self) -> bool: | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| Check if configuration is valid. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||
| bool: Whether the configuration is valid | ||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||
| self._validate() | ||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||
| except InvalidConfigurationError: | ||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,5 @@ | ||||||||||||||||||||||
| pytest>=7.3.1 | ||||||||||||||||||||||
| pytest-asyncio>=0.21.0 | ||||||||||||||||||||||
| requests-mock>=1.9.3 | ||||||||||||||||||||||
| requests>=2.25.1 | ||||||||||||||||||||||
| aiohttp>=3.8.1 | ||||||||||||||||||||||
|
Comment on lines
+1
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify dependency versions for security and compatibility. The development dependencies look appropriate for the testing framework. However, please verify that the specified minimum versions are current and free from known security vulnerabilities. 🏁 Script executed: #!/bin/bash
# Description: Check for latest versions and security advisories for development dependencies
echo "Checking latest versions and security advisories..."
# Check each dependency
for package in pytest pytest-asyncio requests-mock requests aiohttp; do
echo "=== $package ==="
# Get latest version from PyPI
curl -s "https://pypi.org/pypi/$package/json" | jq -r '.info.version // "Version not found"'
# Check for security advisories
gh api graphql -f query="
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: \"$package\") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}" 2>/dev/null || echo "No GitHub API access for security check"
doneLength of output: 5673 Upgrade development dependencies to patched versions for Requests and aiohttp Security scans show no known issues with pytest, pytest-asyncio, or requests-mock, but:
Please bump the minimum versions in requirements-dev.txt accordingly: pytest>=7.3.1
pytest-asyncio>=0.21.0
requests-mock>=1.9.3
-requests>=2.25.1
+requests>=2.32.0
-aiohttp>=3.8.1
+aiohttp>=3.10.11📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| aiohttp>=3.8.4 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify aiohttp version for security and compatibility. The addition of aiohttp is essential for the async client functionality. Please verify that version 3.8.4 is current and free from known security vulnerabilities. 🏁 Script executed: #!/bin/bash
# Description: Check aiohttp latest version and security advisories
echo "Checking aiohttp version and security..."
# Get latest version from PyPI
echo "Latest version:"
curl -s "https://pypi.org/pypi/aiohttp/json" | jq -r '.info.version // "Version not found"'
# Check for security advisories
echo "Security advisories:"
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: PIP, package: "aiohttp") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}' 2>/dev/null || echo "No GitHub API access for security check"Length of output: 3080 Upgrade aiohttp dependency to a secure, supported release The current constraint ( • requirements.txt, line 1
This ensures you install a release that incorporates all known security fixes. 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,102 @@ | ||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||
| from typing import Dict, Any, Optional | ||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Fix import order and add module docstring. The import order should follow Python conventions: standard library imports first, then third-party imports. +"""
+Base API client module providing synchronous HTTP request functionality.
+
+This module contains the BaseAPIClient class for making HTTP requests with
+robust error handling and logging capabilities.
+"""
+import logging
+from typing import Dict, Any, Optional
+
-import logging
import requests
-from typing import Dict, Any, Optional📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[convention] 1-1: Missing module docstring (C0114) [convention] 3-3: standard import "typing.Dict" should be placed before third party import "requests" (C0411) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| class BaseAPIClient: | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| Base API client for making HTTP requests with robust error handling and logging. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| This class provides a standardized interface for making API requests with | ||||||||||||||||||||||||||||
| comprehensive logging and error management. | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def __init__( | ||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||
| base_url: str, | ||||||||||||||||||||||||||||
| timeout: int = 10, | ||||||||||||||||||||||||||||
| log_level: int = logging.INFO | ||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| Initialize the base API client. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||
| base_url (str): Base URL for the API | ||||||||||||||||||||||||||||
| timeout (int, optional): Request timeout in seconds. Defaults to 10. | ||||||||||||||||||||||||||||
| log_level (int, optional): Logging level. Defaults to logging.INFO. | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| self.base_url = base_url | ||||||||||||||||||||||||||||
| self.timeout = timeout | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Configure logging | ||||||||||||||||||||||||||||
| logging.basicConfig( | ||||||||||||||||||||||||||||
| level=log_level, | ||||||||||||||||||||||||||||
| format='%(asctime)s - %(name)s - %(levelname)s - %(message)s' | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
| self.logger = logging.getLogger(self.__class__.__name__) | ||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid configuring logging in class constructor. Calling - # Configure logging
- logging.basicConfig(
- level=log_level,
- format='%(asctime)s - %(name)s - %(levelname)s - %(message)s'
- )
- self.logger = logging.getLogger(self.__class__.__name__)
+ # Configure logger for this instance
+ self.logger = logging.getLogger(self.__class__.__name__)
+ self.logger.setLevel(log_level)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[convention] 32-32: Trailing whitespace (C0303) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def _make_request( | ||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||
| method: str, | ||||||||||||||||||||||||||||
| endpoint: str, | ||||||||||||||||||||||||||||
| params: Optional[Dict[str, Any]] = None, | ||||||||||||||||||||||||||||
| headers: Optional[Dict[str, str]] = None | ||||||||||||||||||||||||||||
| ) -> Dict[str, Any]: | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| Make an HTTP request with robust error handling and logging. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||
| method (str): HTTP method (GET, POST, etc.) | ||||||||||||||||||||||||||||
| endpoint (str): API endpoint | ||||||||||||||||||||||||||||
| params (dict, optional): Query parameters | ||||||||||||||||||||||||||||
| headers (dict, optional): Request headers | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||
| dict: Parsed JSON response | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Raises: | ||||||||||||||||||||||||||||
| ValueError: For invalid method | ||||||||||||||||||||||||||||
| RuntimeError: For network or API errors | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| url = f"{self.base_url}/{endpoint}" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Default headers | ||||||||||||||||||||||||||||
| request_headers = headers or {} | ||||||||||||||||||||||||||||
| request_headers.setdefault('Accept', 'application/json') | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Log request details | ||||||||||||||||||||||||||||
| self.logger.info(f"Sending {method} request to {url}") | ||||||||||||||||||||||||||||
| if params: | ||||||||||||||||||||||||||||
| self.logger.debug(f"Request params: {params}") | ||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use lazy logging formatting. String formatting in logging calls should be lazy to improve performance when the log level filters out the message. - self.logger.info(f"Sending {method} request to {url}")
+ self.logger.info("Sending %s request to %s", method, url)
if params:
- self.logger.debug(f"Request params: {params}")
+ self.logger.debug("Request params: %s", params)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[warning] 67-67: Use lazy % formatting in logging functions (W1203) [warning] 69-69: Use lazy % formatting in logging functions (W1203) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||
| response = requests.request( | ||||||||||||||||||||||||||||
| method=method.upper(), | ||||||||||||||||||||||||||||
| url=url, | ||||||||||||||||||||||||||||
| params=params, | ||||||||||||||||||||||||||||
| headers=request_headers, | ||||||||||||||||||||||||||||
| timeout=self.timeout | ||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Raise an exception for HTTP errors | ||||||||||||||||||||||||||||
| response.raise_for_status() | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Log successful response | ||||||||||||||||||||||||||||
| self.logger.info(f"Received {response.status_code} response") | ||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use lazy logging formatting. - self.logger.info(f"Received {response.status_code} response")
+ self.logger.info("Received %s response", response.status_code)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Pylint (3.3.7)[warning] 84-84: Use lazy % formatting in logging functions (W1203) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return response.json() | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| except requests.exceptions.Timeout: | ||||||||||||||||||||||||||||
| self.logger.error(f"Request to {url} timed out") | ||||||||||||||||||||||||||||
| raise RuntimeError(f"Request to {url} timed out after {self.timeout} seconds") | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+88
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add proper exception chaining and use lazy logging. Exception chaining helps preserve the original traceback for better debugging, and lazy logging improves performance. except requests.exceptions.Timeout:
- self.logger.error(f"Request to {url} timed out")
- raise RuntimeError(f"Request to {url} timed out after {self.timeout} seconds")
+ self.logger.error("Request to %s timed out", url)
+ raise RuntimeError(f"Request to {url} timed out after {self.timeout} seconds") from None
🧰 Tools🪛 Ruff (0.11.9)90-90: Within an (B904) 🪛 Pylint (3.3.7)[convention] 91-91: Trailing whitespace (C0303) [warning] 89-89: Use lazy % formatting in logging functions (W1203) [warning] 90-90: Consider explicitly re-raising using 'except Exception as exc' and 'raise RuntimeError(f'Request to {url} timed out after {self.timeout} seconds') from exc' (W0707) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| except requests.exceptions.HTTPError as e: | ||||||||||||||||||||||||||||
| self.logger.error(f"HTTP error occurred: {e}") | ||||||||||||||||||||||||||||
| raise RuntimeError(f"HTTP error: {e}") | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+92
to
+95
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add proper exception chaining and use lazy logging. except requests.exceptions.HTTPError as e:
- self.logger.error(f"HTTP error occurred: {e}")
- raise RuntimeError(f"HTTP error: {e}")
+ self.logger.error("HTTP error occurred: %s", e)
+ raise RuntimeError(f"HTTP error: {e}") from e📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)94-94: Within an (B904) 🪛 Pylint (3.3.7)[convention] 95-95: Trailing whitespace (C0303) [warning] 93-93: Use lazy % formatting in logging functions (W1203) [warning] 94-94: Consider explicitly re-raising using 'raise RuntimeError(f'HTTP error: {e}') from e' (W0707) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| except requests.exceptions.RequestException as e: | ||||||||||||||||||||||||||||
| self.logger.error(f"Network error occurred: {e}") | ||||||||||||||||||||||||||||
| raise RuntimeError(f"Network error: {e}") | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+96
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add proper exception chaining and use lazy logging. except requests.exceptions.RequestException as e:
- self.logger.error(f"Network error occurred: {e}")
- raise RuntimeError(f"Network error: {e}")
+ self.logger.error("Network error occurred: %s", e)
+ raise RuntimeError(f"Network error: {e}") from e📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)98-98: Within an (B904) 🪛 Pylint (3.3.7)[convention] 99-99: Trailing whitespace (C0303) [warning] 97-97: Use lazy % formatting in logging functions (W1203) [warning] 98-98: Consider explicitly re-raising using 'raise RuntimeError(f'Network error: {e}') from e' (W0707) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| except ValueError as e: | ||||||||||||||||||||||||||||
| self.logger.error(f"JSON parsing error: {e}") | ||||||||||||||||||||||||||||
| raise RuntimeError(f"Could not parse response: {e}") | ||||||||||||||||||||||||||||
|
Comment on lines
+100
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add proper exception chaining and use lazy logging. except ValueError as e:
- self.logger.error(f"JSON parsing error: {e}")
- raise RuntimeError(f"Could not parse response: {e}")
+ self.logger.error("JSON parsing error: %s", e)
+ raise RuntimeError(f"Could not parse response: {e}") from e📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.11.9)102-102: Within an (B904) 🪛 Pylint (3.3.7)[convention] 102-102: Final newline missing (C0304) [warning] 101-101: Use lazy % formatting in logging functions (W1203) [warning] 102-102: Consider explicitly re-raising using 'raise RuntimeError(f'Could not parse response: {e}') from e' (W0707) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # CoinGecko API Client Package | ||
| from .base_client import CoinGeckoBaseClient, BaseAPIConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add module docstring and fix import order.
📝 Committable suggestion
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[error] 3-3: Unable to import 'dotenv'
(E0401)
[warning] 8-8: Unnecessary pass statement
(W0107)
[convention] 4-4: standard import "re" should be placed before third party import "dotenv.load_dotenv"
(C0411)
🤖 Prompt for AI Agents