mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-01-09 08:08:32 +00:00
Implement AI code review suggestions
- Enhanced SSL function documentation with detailed examples and environment variable precedence - Added CA bundle file format validation (.pem, .crt, .cer) with warnings for unsupported formats - Improved error handling with structured solutions and current CA bundle path display - Added comprehensive tests for file format validation and warning scenarios - Enhanced user guidance for SSL certificate configuration issues Addresses feedback from joaomdmoura's AI code review for better documentation, error handling, and path validation as requested. Co-Authored-By: João <joao@crewai.com>
This commit is contained in:
@@ -170,11 +170,22 @@ def get_ssl_verify_config():
|
||||
Example:
|
||||
>>> get_ssl_verify_config()
|
||||
'/path/to/ca-bundle.pem'
|
||||
|
||||
>>> os.environ['REQUESTS_CA_BUNDLE'] = '/custom/ca-bundle.pem'
|
||||
>>> get_ssl_verify_config()
|
||||
'/custom/ca-bundle.pem'
|
||||
"""
|
||||
for env_var in ['REQUESTS_CA_BUNDLE', 'SSL_CERT_FILE', 'CURL_CA_BUNDLE']:
|
||||
ca_bundle = os.environ.get(env_var)
|
||||
if ca_bundle and os.path.isfile(ca_bundle):
|
||||
return ca_bundle
|
||||
if ca_bundle:
|
||||
ca_path = Path(ca_bundle)
|
||||
if ca_path.is_file() and ca_path.suffix in ['.pem', '.crt', '.cer']:
|
||||
return str(ca_path)
|
||||
elif ca_path.is_file():
|
||||
click.secho(f"Warning: CA bundle file {ca_bundle} may not be in expected format (.pem, .crt, .cer)", fg="yellow")
|
||||
return str(ca_path)
|
||||
else:
|
||||
click.secho(f"Warning: CA bundle path {ca_bundle} from {env_var} does not exist", fg="yellow")
|
||||
|
||||
return certifi.where()
|
||||
|
||||
@@ -200,7 +211,10 @@ def fetch_provider_data(cache_file):
|
||||
except requests.exceptions.SSLError as e:
|
||||
click.secho(f"SSL certificate verification failed: {e}", fg="red")
|
||||
click.secho(f"Current CA bundle path: {ssl_config}", fg="yellow")
|
||||
click.secho("Try setting REQUESTS_CA_BUNDLE environment variable to your CA bundle path", fg="yellow")
|
||||
click.secho("Solutions:", fg="cyan")
|
||||
click.secho(" 1. Set REQUESTS_CA_BUNDLE environment variable to your CA bundle path", fg="yellow")
|
||||
click.secho(" 2. Ensure your CA bundle file is in .pem, .crt, or .cer format", fg="yellow")
|
||||
click.secho(" 3. Contact your system administrator for the correct CA bundle", fg="yellow")
|
||||
return None
|
||||
except requests.RequestException as e:
|
||||
click.secho(f"Error fetching provider data: {e}", fg="red")
|
||||
|
||||
@@ -71,6 +71,35 @@ class TestSSLConfiguration:
|
||||
result = get_ssl_verify_config()
|
||||
assert result == '/path/to/certifi/cacert.pem'
|
||||
|
||||
def test_get_ssl_verify_config_file_format_validation(self):
|
||||
"""Test that CA bundle file format validation works correctly."""
|
||||
with tempfile.NamedTemporaryFile(suffix=".pem", delete=False) as temp_file:
|
||||
temp_path = temp_file.name
|
||||
|
||||
try:
|
||||
with patch.dict(os.environ, {"REQUESTS_CA_BUNDLE": temp_path}):
|
||||
result = get_ssl_verify_config()
|
||||
assert result == temp_path
|
||||
finally:
|
||||
os.unlink(temp_path)
|
||||
|
||||
def test_get_ssl_verify_config_unsupported_format_warning(self):
|
||||
"""Test that unsupported file formats still work but show warning."""
|
||||
with tempfile.NamedTemporaryFile(suffix=".txt", delete=False) as temp_file:
|
||||
temp_path = temp_file.name
|
||||
|
||||
try:
|
||||
with patch.dict(os.environ, {"REQUESTS_CA_BUNDLE": temp_path}):
|
||||
with patch('click.secho') as mock_secho:
|
||||
result = get_ssl_verify_config()
|
||||
assert result == temp_path
|
||||
mock_secho.assert_called_with(
|
||||
f"Warning: CA bundle file {temp_path} may not be in expected format (.pem, .crt, .cer)",
|
||||
fg="yellow"
|
||||
)
|
||||
finally:
|
||||
os.unlink(temp_path)
|
||||
|
||||
|
||||
class TestFetchProviderDataSSL:
|
||||
def test_fetch_provider_data_uses_ssl_config(self):
|
||||
@@ -99,7 +128,8 @@ class TestFetchProviderDataSSL:
|
||||
|
||||
assert result is None
|
||||
mock_secho.assert_any_call("SSL certificate verification failed: SSL verification failed", fg="red")
|
||||
mock_secho.assert_any_call("Try setting REQUESTS_CA_BUNDLE environment variable to your CA bundle path", fg="yellow")
|
||||
mock_secho.assert_any_call("Solutions:", fg="cyan")
|
||||
mock_secho.assert_any_call(" 1. Set REQUESTS_CA_BUNDLE environment variable to your CA bundle path", fg="yellow")
|
||||
|
||||
def test_fetch_provider_data_general_request_error(self):
|
||||
cache_file = Path("/tmp/test_cache.json")
|
||||
|
||||
Reference in New Issue
Block a user