fix(openai): repair the annotation that makes litellm unimportable - #35702
fix(openai): repair the annotation that makes litellm unimportable#35702yassin-berriai wants to merge 1 commit into
Conversation
|
|
`BaseOpenAILLM.owns_wrapped_http_client` annotates its parameter with `Optional[Union[httpx.Client, httpx.AsyncClient]]`, but `Union` was never added to the module's typing import. The annotation is evaluated when the class body executes, so `import litellm` raises `NameError: name 'Union' is not defined` and every module that imports litellm dies with it. litellm_internal_staging is red from this: `cd litellm && ruff check .` reports F821 and the whole test matrix fails on import, on every open PR, because CI evaluates the PR merged into the current staging tip. The annotation becomes `httpx.Client | httpx.AsyncClient | None` rather than gaining a `Union` import. requires-python is >=3.10, so PEP 604 unions evaluate at runtime, and UP007 and UP045 both sit at a ceiling of 0 in ruff-strict-budget.json, so importing `Union` would trade the F821 for a budget failure.
cc81916 to
256f1ec
Compare
Greptile SummaryThis PR restores LiteLLM importability by replacing an annotation that references the missing
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/llms/openai/common_utils.py | Replaces the undefined Union reference with an equivalent annotation supported by the package’s Python 3.10+ runtime requirement. |
Reviews (2): Last reviewed commit: "fix(openai): repair the annotation that ..." | Re-trigger Greptile
|
@greptileai please review the current head The 5/5 above is pinned to |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Superseded by #35706, which landed the identical fix ( |
TLDR
Problem this solves:
import litellmraisesNameError: name 'Union' is not definedlitellm_internal_stagingis red on every open PRruff checkreports F821 atllms/openai/common_utils.py:138How it solves it:
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Both runs are a plain
import litellmagainst a worktree at the named commit, nothing mockedBefore the fix,
cd litellm && ruff check .(thelintjob's exact command) reports:Type
🐛 Bug Fix
Changes
BaseOpenAILLM.owns_wrapped_http_clientannotates its parameter withOptional[Union[httpx.Client, httpx.AsyncClient]], butUnionwas never added to the module's typing import. The annotation is evaluated when the class body executes, so theNameErrorfires at import rather than at call time, and every module that imports litellm dies with it.The annotation becomes
httpx.Client | httpx.AsyncClient | Nonerather than gaining aUnionimport.requires-pythonis>=3.10, so PEP 604 unions evaluate at runtime, and both UP007 and UP045 sit at a ceiling of 0 inruff-strict-budget.json, so adding the import would have traded the F821 for a budget failure. That is not hypothetical: the first push of this branch did exactly that and thelintjob went red onUP007: total 67 over limit 0 (this change added 1).That is why
lintand the entire test matrix are failing on open PRs whose diffs contain no Python: CI evaluates the PR merged into the current staging tip, so staging's breakage is attributed to each PR.No test is added. The regression is "the package imports at all", which every existing test in the suite already asserts as a precondition; the whole matrix going from red to green is the assertion.
Final Attestation