Repository navigation
fix(openai): forward project to Batches and Files API clients - #43430
Rainmemery wants to merge 6 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if data.get("project") is None and request.headers.get("OpenAI-Project"): | ||
| data["project"] = request.headers.get("OpenAI-Project") |
There was a problem hiding this comment.
OpenAI-Project, but follow-up handlers ignore it. Without a configured project, retrieval can fail with not-found
Knowledge Base Used: Provider adapters and capabilities
| if data.get("project") is None and request.headers.get("OpenAI-Project"): | ||
| data["project"] = request.headers.get("OpenAI-Project") |
There was a problem hiding this comment.
project form field into request data. The environment default is used instead, or the header wins over the form value
| if data.get("project") is None and request.headers.get("OpenAI-Project"): | ||
| data["project"] = request.headers.get("OpenAI-Project") |
There was a problem hiding this comment.
OpenAI-Project in proxy code. Repository guidance requires provider-specific behavior inside llms/; satisfy that requirement before merging
Rule Used: What: Avoid writing provider-specific code outside of the llms/ directory. Why: This practice ensures better maintainability and reduces complexity over time. Good: ```python # Handle provider-specific logic within llms/vertex_ai/transformation.py ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| file: Required[FileTypes] | ||
| purpose: Required[CREATE_FILE_REQUESTS_PURPOSE] | ||
| expires_after: FileExpiresAfter | None | ||
| project: str | None |
There was a problem hiding this comment.
project fields omit ReadOnly[...]. The repository requires that qualification for TypedDict fields; satisfy it before merging
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| with patch("litellm.llms.openai.openai.OpenAI") as mock_openai: | ||
| mock_openai.return_value.batches.create.return_value = batch_response | ||
| OpenAIBatchesAPI().create_batch( |
There was a problem hiding this comment.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| or "https://api.openai.com/v1" | ||
| ) | ||
| resolved_organization = organization or litellm.organization or os.getenv("OPENAI_ORGANIZATION", None) or None | ||
| resolved_project = project or os.getenv("OPENAI_PROJECT", None) or None |
There was a problem hiding this comment.
resolved_project local has no Final annotation. The repository coding guide requires one for new variables; satisfy it before merging
| resolved_project = project or os.getenv("OPENAI_PROJECT", None) or None | |
| resolved_project: Final = project or os.getenv("OPENAI_PROJECT", None) or None |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
OpenAI project scoping (OpenAI-Project / client project=) is currently dropped by the Batches and Files API paths. First step of plumbing it through: accept 'project' alongside 'organization' in the shared provider-params type, mirroring how organization is declared. Signed-off-by: rain <1504569896@qq.com>
OpenAI project scoping (OpenAI-Project header / client project=) was dropped by the Batches and Files API paths: the OpenAI SDK client was constructed without the project argument, so multi-project keys failed with "Cannot find file file-XXX, or project proj_YYY does not have access to it". - GenericLiteLLMParams accepts project alongside organization - OpenAIFilesAPI / OpenAIBatchesAPI get_openai_client forwards project to OpenAI(**kwargs); every sync method takes a project parameter - batches/files main resolve project from optional_params or OPENAI_PROJECT and pass it through get_openai_credentials - proxy /batches and /files accept project in the body and fall back to the OpenAI-Project header - unit tests cover param/env resolution and client construction Fixes BerriAI#41803
…icts project is a litellm param, not part of the OpenAI request bodies: it is absorbed by GenericLiteLLMParams from kwargs and forwarded to the client constructor. Declaring it on CreateFileRequest / LiteLLMBatchCreateRequest made files.create(**create_file_data) type-check against a key the OpenAI SDK never accepts (+2 reportCallIssue over the repo budget). Also regenerates the dashboard schema.d.ts for the LiteLLM_Params / updateLiteLLMParams project field.
…cycle behind an opt-in Address review: project now resolves once via apply_openai_project_to_data and applies to create/retrieve/list/cancel/get/delete for both batches and files, closing the gap where a file or batch created in a project could not be read back. create_file now accepts the project form field. Client-supplied project is gated by general_settings.forward_openai_project (default drop), mirroring forward_openai_org_id, so it can no longer override the deployment credential. Add resolved_project Final annotation and functional proxy regression tests.
…hema.d.ts The create_file project form field changes the generated OpenAPI spec, so the dashboard's schema.d.ts must carry the matching optional project on the three Body_create_file_* components or the 'Verify schema.d.ts matches the proxy OpenAPI spec' job fails.
57d2be2 to
57fd857
Compare
|
Heads-up: the red I've rebased the branch onto current Fresh CI should come back green on the rebased head. |
After rebasing onto main (which removed the LIT002 mutable-construction rule in BerriAI#43971), the '# mutable-ok' annotation on this line suppressed nothing and tripped the LIT013 stale-suppression gate. The copy is a deliberate rebinding, so annotate it as one.
|
One follow-up on the |
This branch has not been deployed
1 participant
Problem
OpenAI project scoping is currently dropped by the Batches and Files API paths. The OpenAI SDK client used by
openai_batches_instance/openai_files_instanceis constructed fromapi_key,api_base,organization, ... — but neverproject— so with a multi-project key, requests hit the wrong project and fail with:organizationis resolved fromoptional_params/litellm.organization/OPENAI_ORGANIZATIONand forwarded on every call;project(theOpenAI-Projectheader, the client-levelproject=kwarg, orOPENAI_PROJECTenv) has no equivalent path, even though the OpenAI SDK supports a client-levelprojectargument.Changes
GenericLiteLLMParams): acceptprojectalongsideorganization, soproject=...reaches the provider params fromlitellm.create_batch/litellm.file_*calls, the router, and proxy bodies.OpenAIFilesAPI/OpenAIBatchesAPI):get_openai_clienttakesprojectand forwards it toOpenAI(**kwargs)/AsyncOpenAI(**kwargs); every sync entry method (create_batch,retrieve_batch,cancel_batch,list_batches,create_file,file_content,file_content_streaming,retrieve_file,delete_file,list_files) takes aprojectparameter.Nonevalues are skipped by the existinglocals()passthrough, so no behavior change when project is unset.projectfromoptional_params.projectorOPENAI_PROJECT(mirroring theorganization/OPENAI_ORGANIZATIONresolution) and pass it through; files-side goes viaOpenAICredentialswhich now carriesproject./v1/batchesand/v1/filesacceptprojectin the request body (flows through the generic params), and fall back to theOpenAI-Projectrequest header when the body does not set one. Body value wins.Testing
New unit tests (
tests/batches_tests/test_openai_project_passthrough.py, 8 cases):get_openai_credentialscarries explicitprojectand falls back toOPENAI_PROJECTenvOpenAIBatchesAPI.create_batch/OpenAIFilesAPI.retrieve_fileforwardprojectto theOpenAIclient constructor; whenproject/organizationareNonethey are omitted entirely (no behavior change for existing users)litellm.create_batch(project=...)resolves throughGenericLiteLLMParamsinto the provider instance calllitellm.create_batch()with onlyOPENAI_PROJECTenv set picks it upfiles_main.file_retrieve(project=...)passes throughExisting batches/files unit tests still pass (the remaining e2e failures in
test_openai_batches_and_files.pyrequire liveOPENAI_API_KEYcredentials and fail identically without this change).Fixes #41803