-
Notifications
You must be signed in to change notification settings - Fork 10
Enhance Configuration Management: Flexible and Robust CoinGecko API Configuration #20
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
Open
HermanKoii
wants to merge
6
commits into
Prometheus-Swarm:main
Choose a base branch
from
HermanKoii:817b2288-cdda-4839-9c30-95bdebbe9335-merged
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b32ed71
Start draft PR
HermanKoii d1bbfd7
Add comprehensive .gitignore file
HermanKoii 6ff37b0
Create configuration loading utility
HermanKoii 6efe1fb
Add tests for configuration loading utility
HermanKoii 188eacb
Start draft PR
HermanKoii b62437c
Merged branch pr-5-HermanKoii-prometheus-test for PR https://github.c…
HermanKoii File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,48 +1,46 @@ | ||
| .venv | ||
| .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/ | ||
| # 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 | ||
| MANIFEST | ||
|
|
||
| # Environment and configuration | ||
| .env | ||
| .venv | ||
| env/ | ||
| venv/ | ||
| ENV/ | ||
| env.bak/ | ||
| venv.bak/ | ||
|
|
||
| # Ignore Data | ||
| data/* | ||
|
|
||
| # Testing | ||
| .pytest_cache/ | ||
| .coverage | ||
| htmlcov/ | ||
|
|
||
| venv | ||
| # IDE | ||
| .vscode/ | ||
| .idea/ | ||
| *.swp | ||
| *.swo | ||
|
|
||
| **/venv/ | ||
| # OS | ||
| .DS_Store | ||
| Thumbs.db |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import os | ||
| from typing import Optional, Dict, Any | ||
| from dotenv import load_dotenv | ||
|
|
||
|
|
||
| class ConfigurationError(Exception): | ||
| """Custom exception for configuration loading errors.""" | ||
| pass | ||
|
|
||
|
|
||
| def load_config(config_file: Optional[str] = None) -> Dict[str, Any]: | ||
| """ | ||
| Load configuration from environment variables or a specified config file. | ||
|
|
||
| Args: | ||
| config_file (Optional[str]): Path to an optional configuration file. | ||
|
|
||
| Returns: | ||
| Dict[str, Any]: Loaded configuration dictionary. | ||
|
|
||
| Raises: | ||
| ConfigurationError: If required configuration is missing or invalid. | ||
| """ | ||
| # Load .env file if specified or default .env exists | ||
| if config_file: | ||
| load_dotenv(config_file) | ||
| else: | ||
| load_dotenv() | ||
|
|
||
| # Configuration dictionary to store loaded settings | ||
| config = { | ||
| 'api_base_url': os.getenv('COINGECKO_API_BASE_URL', 'https://api.coingecko.com/api/v3'), | ||
| 'api_key': os.getenv('COINGECKO_API_KEY', ''), | ||
| 'request_timeout': int(os.getenv('COINGECKO_REQUEST_TIMEOUT', 30)), | ||
| 'max_retries': int(os.getenv('COINGECKO_MAX_RETRIES', 3)), | ||
| 'rate_limit_delay': int(os.getenv('COINGECKO_RATE_LIMIT_DELAY', 1)) | ||
| } | ||
|
|
||
| # Validate critical configuration | ||
| _validate_config(config) | ||
|
|
||
| return config | ||
|
|
||
|
|
||
| def _validate_config(config: Dict[str, Any]) -> None: | ||
| """ | ||
| Validate the loaded configuration. | ||
|
|
||
| Args: | ||
| config (Dict[str, Any]): Configuration dictionary to validate. | ||
|
|
||
| Raises: | ||
| ConfigurationError: If configuration is invalid. | ||
| """ | ||
| # Basic validation rules | ||
| if not config['api_base_url']: | ||
| raise ConfigurationError("Invalid or missing API base URL") | ||
|
|
||
| if config['request_timeout'] <= 0: | ||
| raise ConfigurationError("Request timeout must be a positive integer") | ||
|
|
||
| if config['max_retries'] < 0: | ||
| raise ConfigurationError("Max retries cannot be negative") | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import os | ||
| import pytest | ||
| from src.config import load_config, ConfigurationError | ||
|
|
||
|
|
||
| def test_default_config(monkeypatch): | ||
| """Test default configuration loading.""" | ||
| monkeypatch.delenv('COINGECKO_API_BASE_URL', raising=False) | ||
| monkeypatch.delenv('COINGECKO_API_KEY', raising=False) | ||
| monkeypatch.delenv('COINGECKO_REQUEST_TIMEOUT', raising=False) | ||
| monkeypatch.delenv('COINGECKO_MAX_RETRIES', raising=False) | ||
| monkeypatch.delenv('COINGECKO_RATE_LIMIT_DELAY', raising=False) | ||
|
|
||
| config = load_config() | ||
| assert config['api_base_url'] == 'https://api.coingecko.com/api/v3' | ||
| assert config['api_key'] == '' | ||
| assert config['request_timeout'] == 30 | ||
| assert config['max_retries'] == 3 | ||
| assert config['rate_limit_delay'] == 1 | ||
|
|
||
|
|
||
| def test_custom_config(monkeypatch): | ||
| """Test loading custom configuration from environment variables.""" | ||
| monkeypatch.setenv('COINGECKO_API_BASE_URL', 'https://custom-api.com') | ||
| monkeypatch.setenv('COINGECKO_API_KEY', 'test_key') | ||
| monkeypatch.setenv('COINGECKO_REQUEST_TIMEOUT', '60') | ||
| monkeypatch.setenv('COINGECKO_MAX_RETRIES', '5') | ||
| monkeypatch.setenv('COINGECKO_RATE_LIMIT_DELAY', '2') | ||
|
|
||
| config = load_config() | ||
| assert config['api_base_url'] == 'https://custom-api.com' | ||
| assert config['api_key'] == 'test_key' | ||
| assert config['request_timeout'] == 60 | ||
| assert config['max_retries'] == 5 | ||
| assert config['rate_limit_delay'] == 2 | ||
|
|
||
|
|
||
| def test_invalid_timeout_config(monkeypatch): | ||
| """Test configuration validation for negative timeout.""" | ||
| monkeypatch.setenv('COINGECKO_REQUEST_TIMEOUT', '-10') | ||
|
|
||
| with pytest.raises(ConfigurationError, match="Request timeout must be a positive integer"): | ||
| load_config() | ||
|
|
||
|
|
||
| def test_empty_base_url_config(monkeypatch): | ||
| """Test configuration validation for empty base URL.""" | ||
| monkeypatch.setenv('COINGECKO_API_BASE_URL', '') | ||
|
|
||
| with pytest.raises(ConfigurationError, match="Invalid or missing API base URL"): | ||
| load_config() | ||
|
|
||
|
|
||
| def test_negative_retries_config(monkeypatch): | ||
| """Test configuration validation for negative retries.""" | ||
| monkeypatch.setenv('COINGECKO_MAX_RETRIES', '-3') | ||
|
|
||
| with pytest.raises(ConfigurationError, match="Max retries cannot be negative"): | ||
| load_config() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 error handling for invalid numeric environment variables.
The current implementation will raise a
ValueErrorif environment variables contain non-numeric values. Consider wrapping the int conversions in try-catch blocks for better error messages.Add this helper function before the
load_configfunction:🧰 Tools
🪛 Pylint (3.3.7)
[warning] 34-34: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[warning] 35-35: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[warning] 36-36: os.getenv default type is builtins.int. Expected str or None.
(W1508)
🤖 Prompt for AI Agents
🛠️ Refactor suggestion
Fix type safety issues with os.getenv defaults.
The current implementation passes integer defaults to
os.getenv, which expects string defaults. This could cause type-related issues.📝 Committable suggestion
🧰 Tools
🪛 Pylint (3.3.7)
[warning] 34-34: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[warning] 35-35: os.getenv default type is builtins.int. Expected str or None.
(W1508)
[warning] 36-36: os.getenv default type is builtins.int. Expected str or None.
(W1508)
🤖 Prompt for AI Agents