From 2f75145cac30b95b838922f1aa9abee9f647d998 Mon Sep 17 00:00:00 2001 From: Pravit Ampapathini Date: Thu, 13 Aug 2026 15:17:01 -0700 Subject: [PATCH] refactor(seed): remove obsolete requests import handling in load_from_api requests is a core dependency, so the ImportError arm of the except (ImportError, OSError) block in SeedDataManager.load_from_api() cannot realistically fire. The OSError arm was actively harmful: requests.exceptions.RequestException subclasses OSError, so connection errors, timeouts, and raise_for_status() failures were all reported as 'requests library not available. Install with: pip install requests'. Hoist the lazy import to module scope, matching the ingest stack, and drop the block so genuine failures fall through to the existing ProcessingError('Failed to load from API: ...') with the cause chained. Update the docstring Raises section accordingly and add regression tests covering connection, timeout, HTTP status, and JSON parse failures. Closes #949 --- semantica/seed/seed_manager.py | 16 +++++----- tests/test_seed_manager.py | 55 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/semantica/seed/seed_manager.py b/semantica/seed/seed_manager.py index 0d3bb046..a9cac4d0 100644 --- a/semantica/seed/seed_manager.py +++ b/semantica/seed/seed_manager.py @@ -38,6 +38,12 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Union +# ``requests`` is a core dependency (see ``dependencies`` in pyproject.toml), so +# it is imported at module scope like the rest of the ingest stack. A missing +# install is a packaging problem, not a runtime condition callers can recover +# from, so ``load_from_api`` deliberately carries no ImportError fallback. +import requests + from ..utils.exceptions import ProcessingError, ValidationError from ..utils.helpers import read_json_file, write_json_file from ..utils.logging import get_logger @@ -466,8 +472,8 @@ def load_from_api( List of loaded data records as dictionaries Raises: - ProcessingError: If API request fails, response parsing fails, or - requests library is not available + ProcessingError: If the API request fails (connection error, + timeout, non-2xx status) or the response cannot be parsed Example: >>> records = manager.load_from_api( @@ -478,8 +484,6 @@ def load_from_api( ... ) """ try: - import requests - # Build full URL if endpoint: full_url = f"{api_url.rstrip('/')}/{endpoint.lstrip('/')}" @@ -530,10 +534,6 @@ def load_from_api( self.logger.info(f"Loaded {len(records)} records from API: {full_url}") return records - except (ImportError, OSError): - raise ProcessingError( - "requests library not available. Install with: pip install requests" - ) except Exception as e: raise ProcessingError(f"Failed to load from API: {e}") from e diff --git a/tests/test_seed_manager.py b/tests/test_seed_manager.py index 0f73b40d..098982c2 100644 --- a/tests/test_seed_manager.py +++ b/tests/test_seed_manager.py @@ -5,6 +5,9 @@ import csv from pathlib import Path from unittest.mock import MagicMock, patch + +import requests + from semantica.seed.seed_manager import SeedDataManager, SeedDataSource, SeedData from semantica.utils.exceptions import ProcessingError @@ -164,6 +167,58 @@ def test_load_from_api(mock_get, seed_manager): assert records[0]["entity_type"] == "User" mock_get.assert_called_once() +# requests.exceptions.RequestException subclasses OSError, so network failures used +# to be reported as "requests library not available" by the obsolete ImportError / +# OSError handler. They must surface the real cause instead. +@pytest.mark.parametrize( + "error", + [ + requests.exceptions.ConnectionError("connection refused"), + requests.exceptions.Timeout("timed out"), + requests.exceptions.HTTPError("500 Server Error"), + ], +) +@patch("requests.get") +def test_load_from_api_request_failure_reports_real_cause(mock_get, error, seed_manager): + mock_get.side_effect = error + + with pytest.raises(ProcessingError) as excinfo: + seed_manager.load_from_api(api_url="http://api.example.com", endpoint="users") + + message = str(excinfo.value) + assert "Failed to load from API" in message + assert str(error) in message + assert "requests library not available" not in message + assert excinfo.value.__cause__ is error + +@patch("requests.get") +def test_load_from_api_http_status_error_reports_real_cause(mock_get, seed_manager): + http_error = requests.exceptions.HTTPError("404 Client Error: Not Found") + mock_response = MagicMock() + mock_response.raise_for_status.side_effect = http_error + mock_get.return_value = mock_response + + with pytest.raises(ProcessingError) as excinfo: + seed_manager.load_from_api(api_url="http://api.example.com", endpoint="users") + + message = str(excinfo.value) + assert "404 Client Error: Not Found" in message + assert "requests library not available" not in message + mock_response.json.assert_not_called() + +@patch("requests.get") +def test_load_from_api_invalid_json_reports_real_cause(mock_get, seed_manager): + mock_response = MagicMock() + mock_response.json.side_effect = ValueError("Expecting value: line 1 column 1") + mock_get.return_value = mock_response + + with pytest.raises(ProcessingError) as excinfo: + seed_manager.load_from_api(api_url="http://api.example.com") + + message = str(excinfo.value) + assert "Failed to load from API" in message + assert "Expecting value" in message + def test_load_source(seed_manager, temp_data_dir): json_file = temp_data_dir / "source.json" with open(json_file, "w") as f: