feat: make datasets and litellm optional dependencies#1972
Conversation
|
@astefanutti is attempting to deploy a commit to the Harbor Framework Team on Vercel. A member of the Team first needs to authorize it. |
|
Enjoy a better diff viewing experience by clicking one of these URLs: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
66f5470 to
e8ce15b
Compare
|
This is an interesting insight. They really don't like big changes though. |
e8ce15b to
cd3e76e
Compare
| def _build_provider_model_names(): | ||
| try: | ||
| import litellm | ||
| except ImportError: | ||
| raise ImportError( | ||
| "litellm is required for model provider detection. " | ||
| "Install it with: pip install 'harbor[litellm]'" | ||
| ) from None |
There was a problem hiding this comment.
🚩 Agents like openhands, mini_swe_agent, swe_agent now require harbor[litellm] at runtime
Files like src/harbor/agents/installed/mini_swe_agent.py:13, openhands.py:11, and swe_agent.py:13 have top-level from harbor.agents.utils import get_api_key_var_names_from_model_name. Importing this function is safe (no litellm import triggered at import time after this PR), but calling it will raise ImportError if litellm is not installed. Before this PR, litellm was always available. This is a behavioral change for users who install harbor without [litellm] — these agents will fail at runtime with an ImportError when trying to resolve API keys. The error message from _build_provider_model_names() at src/harbor/agents/utils.py:52-54 is clear about how to fix it, so this is acceptable but worth noting in release documentation.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@alexgshaw I've rebased on top of #2023 which happens to make this PR much simpler as you hinted during the community call, thanks! |
|
Huh, is the datasets dependency even being used anywhere? I don't see any changed references in your PR. |
|
Yes that PR just moves the dependency from core to optional in The datasets is only used for HuggingFace Hub export, not the eval loop. |
cd3e76e to
3efb6b0
Compare
Move three heavy dependencies from core to optional extras, reducing base install from 544MB to 39MB (93% reduction): - claude-agent-sdk (219MB) → harbor[claude-sdk] - datasets + pyarrow + pandas + numpy (180MB) → harbor[datasets] - litellm (55MB) → harbor[litellm] All three are peripheral to the core eval loop (trial runner, orchestrators, verifier). Imports are converted to function-level lazy loading with clear error messages pointing users to the right extra when a feature requires an optional dependency. The dev dependency group and `all` meta-extra include all three, so development workflows are unchanged.
The model_info registration in __init__ was unguarded — would crash if litellm isn't installed and model_info kwarg is passed.
…r in annotator - Add try/except around get_llm_provider import with user-friendly error - Move import check before @Retry decorator so ImportError fails immediately instead of retrying 3 times with exponential backoff
3efb6b0 to
71e8936
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The datasets library is an optional extra, but several code paths in traces_utils.py and sweeps.py imported it bare, surfacing a raw ModuleNotFoundError (or a generic RuntimeError) instead of the install-hint pattern used everywhere else in this PR. Add a `_require_datasets()` helper in traces_utils.py and route the runtime import sites through it; wrap the sweeps.py export import with the same try/except. Missing datasets now raises a clear ImportError pointing to `pip install 'harbor[datasets]'`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I've actually added some import guards for datasets as per Devin's review feedback. @alexgshaw I think it's ready for another review when you can. |
Summary
datasets+ pyarrow/pandas/numpy (180MB), andlitellm(55MB) from core dependencies to optional extrasharbor[claude-sdk],harbor[datasets],harbor[litellm]Test plan
harbor --helpandharbor run --helpwork in a slim venv without optional depsdevgroup andallmeta-extra include all three)harbor run -t hello-world/hello-worldCloses #1971