mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-09 22:31:41 +00:00
3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
yuneng-jiang
|
6a0d03914c
|
test: drop the cwd-relative sys.path.insert calls from the test suite (#37802)
* test: drop the cwd-relative sys.path.insert calls from the test suite
TQ003 stands at 1,077 across 1,058 files, and 1,015 of them are the same shape:
sys.path.insert(0, os.path.abspath("../..")) and its deeper siblings. The
argument resolves against the working directory rather than the file, so from
the repo root, where every job runs pytest, it inserts the directory two levels
above the checkout. It has never pointed at litellm. The package is installed
into the environment anyway, which is what actually makes the import work, and
what the rule's message has said all along.
Removing them leaves 1,634 imports of sys and os with no remaining reference,
and those go too, except where another test module imports the name back out of
the file. The rest of TQ003 is 62 call sites that resolve against __file__ or a
variable, which are a different question and are left alone.
Collection is identical either way: 45,871 tests and the same 51 pre-existing
collection errors before and after, and ruff reports no new undefined name.
* test: drop the duplicate imports the sys.path sweep exposed to F811
* test(pre-call-utils): restore the os import the new bedrock tests need
|
||
|
ryan-crabbe-berri
|
680bcfd8aa
|
test(lint): ban blind pytest.raises(Exception) with ruff B017 (#37731)
* test(lint): ban blind pytest.raises(Exception) with ruff B017 A bare pytest.raises(Exception) accepts whatever the body throws. The TypeError a refactor introduces satisfies it exactly as well as the rejection the test was written for, so the crash reads as a pass and the test never goes red. All 111 existing sites are narrowed here. A runtime probe recorded the concrete exception each one actually catches, and each site now names that type. Where the code under test genuinely raises a bare Exception, the site pins a stable slice of the message with match= instead. Two sites tell on themselves. The shared responses-API cancel test raises "custom_llm_provider is required but passed as None" rather than talking to a provider at all, because cancel_responses takes a provider, not a model. And test_bedrock_guardrails_with_streaming was the only test in its file still passing without AWS credentials, because the NoCredentialsError boto3 raised long before the guardrail ran satisfied the blind raises. * fix(test): widen the openai batch-dispatch assertion to OpenAIError The narrowed NotFoundError only holds where OPENAI_API_KEY is set. Without one the SDK raises OpenAIError while building the client, long before any 404, so CI went red. OpenAIError covers both and still rejects a TypeError from a refactor. |
||
|
Dawei Gu
|
0751886680
|
feat(batch-job): bedrock batch model invocation job retrieval (#26834)
* feat(bedrock): support retrieve for model-invocation-job batch ARNs `bedrock.retrieve_batch` previously only handled `:async-invoke/` ARNs (Twelve Labs Marengo embeddings). The `:model-invocation-job/` ARNs returned by `CreateModelInvocationJob` (the bulk batch inference API behind `bedrock.create_batch`) fell through and returned a misleading data-plane error, leaving created jobs unretrievable through the LiteLLM batches API. The two ARN families live on different AWS service endpoints (`bedrock-runtime` data plane vs `bedrock` control plane), so they need distinct handlers. This adds: * `BedrockBatchesHandler._handle_model_invocation_job_status` — calls the control plane via boto3 (`bedrock:GetModelInvocationJob`), reusing `BaseAWSLLM.get_credentials` for credential resolution so model_list / env / role-assumption configs continue to apply. The response is reshaped into a `LiteLLMBatch` with the same status mapping `transform_create_batch_response` already uses. * Output-file-URI prediction. Bedrock surfaces the user-supplied `s3OutputDataConfig.s3Uri` *prefix* in `GetModelInvocationJob`, but results actually land at `<prefix>/<job-id>/<basename(input)>.out`. We compute that single-file URI client-side and surface it as `output_file_id`, so OpenAI-style `client.files.content(...)` works without an extra `ListObjectsV2` round-trip. The bare prefix stays in metadata for callers that want the manifest. * Dispatch in `litellm/batches/main.py` for the new ARN family, alongside the existing async-invoke branch. * Unit tests covering ARN parsing, output-URI prediction (incl. edge cases), the full status mapping, region resolution precedence, and failure-message propagation. Note: `request_counts` is intentionally `(0, 0, 0)` — `GetModelInvocationJob` does not report per-record counts; getting accurate numbers requires parsing `manifest.json.out` from the output S3 prefix, which is left to callers. Made-with: Cursor * fix(bedrock): address PR feedback on model-invocation-job retrieve Addresses Greptile P2 findings on #26834: 1. Use the bare job id (not the full ARN) when constructing the `api_base` URL for `pre_call` logging. Passing the full ARN double- counts the `model-invocation-job/` segment and embeds colons in the path, producing misleading log lines. 2. Drop the `or output_prefix` fallback when `_predict_output_file_uri` returns None. A bare prefix is not a downloadable object and surfacing it as `output_file_id` re-creates the very NoSuchKey bug this handler exists to fix. The bare prefix is still preserved in `metadata["output_s3_uri"]` for callers that want to do their own S3 listing or read `manifest.json.out`. `metadata["output_file_uri"]` uses "" rather than None to satisfy the OpenAI Batch metadata schema (`dict[str, str]`); callers should branch on the typed `output_file_id` field instead. Also expands test coverage on the new code path: - new "stay None" regression test for the prediction-fail case - pre_call/post_call logging hook assertions (incl. the bare-id URL) - explicit cancelled_at / expired_at coverage - _to_epoch type-handling matrix and the boto3 ImportError branch - defensive _extract_region_from_bedrock_arn exception path - empty-basename case for _predict_output_file_uri Patch coverage on the changed lines is now 100% (the only remaining uncovered lines in the file belong to the pre-existing `_handle_async_invoke_status` method, which this PR does not touch). Made-with: Cursor * test(bedrock): cover retrieve_batch dispatch for both ARN families Codecov flagged 8 uncovered lines on `litellm/batches/main.py` after this PR refactored the Bedrock dispatch into a single guard with two sub-branches (`async-invoke` + `model-invocation-job`). Existing tests exercised the handlers directly but not the dispatch in `main.py`. Adds `tests/test_litellm/batches/test_retrieve_batch_bedrock_dispatch.py` with 6 mocked tests that exercise `litellm.retrieve_batch` end-to-end for the dispatch logic: - async-invoke ARN routes to `_handle_async_invoke_status` - async-invoke ARN with no region falls back to "us-east-1" (preserves prior behavior on this branch) - model-invocation-job ARN routes to the new `_handle_model_invocation_job_status` handler - model-invocation-job ARN with no region forwards None (so the new handler can sniff region from the ARN itself, rather than getting silently routed to us-east-1) - unrelated bedrock ARN family falls through to the generic provider-config retrieve path (neither special handler invoked) - non-bedrock batch ids skip the bedrock dispatch entirely Both handlers are mocked at the import site so the tests don't hit AWS — the focus here is purely the new dispatch logic in main.py. Co-authored-by: Cursor <cursoragent@cursor.com> * test(bedrock): move retrieve_batch dispatch test to tests/test_litellm/ The dispatch test landed under `tests/test_litellm/batches/`, a new directory that no upstream `test-unit-*.yml` workflow's `test-path` allow-list includes. As a result, the test was never executed in CI and codecov reported `litellm/batches/main.py` patch coverage at 11.11% (8 lines uncovered) — the lines belonging to this PR's dispatch refactor itself. Move the file up one level so it matches the `tests/test_litellm/test_*.py` glob that `test-unit-misc.yml` already runs, and adjust `sys.path.insert` for the new depth. The companion handler tests under `tests/test_litellm/llms/bedrock/batches/test_handler.py` are unaffected — they're picked up by the `llms` directory in `test-unit-llm-providers.yml`. Made-with: Cursor --------- Co-authored-by: Cursor <cursoragent@cursor.com> |