refactor(seed): remove obsolete requests import handling in load_from_api

[pravit-amp]

Description

SeedDataManager.load_from_api() ended with except (ImportError, OSError) raising ProcessingError("requests library not available. Install with: pip install requests"). Both arms are wrong:

• ImportError can't realistically fire. requests is a core dependency (dependencies in pyproject.toml), not an optional extra, so the lazy import it was guarding served no purpose.
• OSError was actively harmful. requests.exceptions.RequestException subclasses OSError, so ConnectionError, Timeout, and the HTTPError from raise_for_status() were all caught by that arm and reported as "requests library not available" — telling users to reinstall a library that was already installed, while hiding the real failure and dropping the exception chain.

This removes the block and hoists the import to module scope, matching how the rest of the ingest stack (api_ingestor, web_ingestor, ssrf, …) imports requests. Genuine failures now fall through to the pre-existing ProcessingError(f"Failed to load from API: {e}") from e, which was already there and is unchanged.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Performance improvement
• Code refactoring

Filed as a refactor, but the OSError overlap made it a real behavior bug, so both are ticked.

Related Issues

Closes #949

Changes Made

• Removed the obsolete except (ImportError, OSError) block from SeedDataManager.load_from_api(); request, parsing, and network failures now surface through the existing Failed to load from API: {e} path with from e chaining intact.
• Hoisted import requests out of the try body to module scope, with a comment recording why there is no import guard, so the lazy-import-plus-fallback pattern doesn't get reintroduced.
• Corrected the load_from_api docstring, which advertised ProcessingError "if ... requests library is not available".
• Added regression tests for ConnectionError, Timeout, HTTPError, a failing raise_for_status(), and a malformed JSON body — each asserting the real cause reaches the caller and the "requests library not available" text does not.

Testing

• Tested locally
• Added tests for new functionality
• Package builds successfully (python -m build)

Build not run — the change touches no packaging metadata, only a module body and a test file.

The new tests were verified to fail against the pre-change code (4 of the 5 fail; the malformed-JSON case passes before and after, since ValueError was never caught by the removed block — it's there to pin that behavior).

Test Commands

pytest tests/test_seed_manager.py tests/seed/ -q

28 passed. black/flake8 report diffs on both touched files, but those are pre-existing on main (unrelated lines); the added lines are clean.

Documentation

• Updated relevant documentation
• Added code examples if applicable
• Updated API reference if adding new APIs
• Updated cookbook if adding new examples
• No documentation changes needed

The only stale doc was the method's own Raises: section, corrected in this PR.

Breaking Changes

Breaking Changes: No

Every failure mode still raises ProcessingError from load_from_api(); only the message text changes, and only for cases that were previously mislabeled. Anything matching on the literal string "requests library not available" would need updating — a grep found no such callers or tests in the repo.

Additional Notes

Two things for reviewers:

1. Overlap with fix seed SSRF #942. That PR replaces requests.get() here with request_with_ssrf_guard() and touches the same region, so whichever lands second needs a small rebase. refactor: remove obsolete requests import exception handling in SeedDataManager #949 was filed as a pre-existing cleanup, explicitly not a blocker for the SSRF fix, so this is based on main rather than on that branch.
2. Sibling left out of scope. load_from_database() (same class, ~50 lines up) has the identical except (ImportError, OSError) shape, mislabeling real DB and socket errors as "Database ingestion module not available". Deliberately not touched here to keep the PR scoped to the linked issue — happy to fix it in a follow-up, or in this PR if you'd prefer them together.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix load_from_api error reporting by removing obsolete requests import guard

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Remove misleading ImportError/OSError handling that masked real requests failures.
• Import requests at module scope and update load_from_api Raises docstring.
• Add regression tests asserting connection/timeout/HTTP/JSON errors surface with proper chaining.

Diagram

graph TD
  T["tests/test_seed_manager.py"] --> L["SeedDataManager.load_from_api"] --> R["requests.get + raise_for_status"] --> API{{"External API"}}
  L --> E["ProcessingError (from e)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Catch requests.RequestException explicitly

• ➕ Allows a more tailored error message for network/HTTP failures
• ➕ Avoids wrapping unrelated exceptions (e.g., JSON parsing) into the same bucket if desired
• ➖ More branching without much functional benefit since the existing catch already chains the cause
• ➖ Still needs careful ordering to preserve original exception context

2. Keep a lazy import but only handle ImportError

• ➕ Preserves a clear message if packaging is broken or dependencies are missing
• ➖ Unnecessary for a core dependency; adds noise and invites reintroducing overly-broad exception handling

3. Fix load_from_database sibling pattern in the same PR

• ➕ Eliminates the same OSError-masking class of bug for database ingestion
• ➕ Reduces future duplicated work
• ➖ Broadens scope beyond the linked issue and increases review surface area

Recommendation: Current approach is the best fit for a core dependency: import requests at module scope and remove the overly-broad (ImportError, OSError) handler so real request failures flow into the existing ProcessingError("Failed to load from API: ...") from e path. Consider a small follow-up PR to apply the same correction to load_from_database(), which still uses the problematic (ImportError, OSError) pattern.

Files changed (2) +63 / -8

Bug fix (1) +8 / -8

seed_manager.pyHoist requests import and stop masking API request failures +8/-8

Hoist requests import and stop masking API request failures

• Imports requests at module scope with an explanatory comment, removing the dead/unsafe lazy-import fallback. Deletes the (ImportError, OSError) handler so request/network/HTTP errors are wrapped by the existing ProcessingError("Failed to load from API: ...") with proper exception chaining. Updates the load_from_api docstring to reflect real failure modes.

semantica/seed/seed_manager.py

Tests (1) +55 / -0

test_seed_manager.pyAdd regression coverage for request/HTTP/JSON failures in load_from_api +55/-0

Add regression coverage for request/HTTP/JSON failures in load_from_api

• Adds tests covering ConnectionError, Timeout, and HTTPError (including raise_for_status()) to ensure the true cause reaches callers and the old misleading "requests library not available" text is not emitted. Adds a malformed-JSON test to pin the existing parse-failure behavior.

tests/test_seed_manager.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗