fix(files): support filename for stream and byte-array multipart uploads - #740
fix(files): support filename for stream and byte-array multipart uploads#740bsaptarshi wants to merge 2 commits into
Conversation
|
Hi @TomerAberbach, Following up on Issue #284. I have added this PR to include default filename fallbacks (file.bin) for InputStream and byte[] overloads, plus new explicit filename overloads. This should resolve the 400 errors for anonymous uploads while remaining backward compatible. Please help review the changes! |
|
One more thing to add regarding the test failures. I confirmed they are unrelated to this PR and were already present on I ran full tests on both:
The same baseline admin/org failures appear in both runs. |
|
Can you or someone take a look at this PR? If it's no.longer required feel free to update the issue and I'll retract this. Not sure what the plan is for this one. |
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Two blockers:
-
openai-java-core/src/main/kotlin/com/openai/models/files/FileCreateParams.kt:156—file.binis not a safe default for this endpoint. The model documentation immediately above says both Fine-tuning and Batch only accept.jsonl, and the README’s anonymous-stream example usesFilePurpose.FINE_TUNE. With this change, that example sendsfilename="file.bin", so the API still rejects the request. The new tests even pairFilePurpose.BATCHwithfile.bin, but only inspect the localMultipartField, masking that the resulting request is invalid. Different purposes require different extensions, so there is no universal fallback here. Please keep the explicit filename overloads, avoid presentingfile.binas a safe default for/files, and update the README/examples to use the new filename-aware overload (or otherwise make callers supply a purpose-appropriate filename). -
openai-java-core/src/test/kotlin/com/openai/models/files/FileCreateParamsTest.kt:107— the branch fails the repository formatter../gradlew :openai-java-core:lintKotlin --no-daemon --no-parallelreports this file; the chained builder call needs ktfmt formatting.
Validation: the two targeted FileCreateParamsTest suites pass, and git diff --check passes, but :openai-java-core:lintKotlin fails as described.
|
Thanks for reviewing @jbeckwith-oai I ack and agree on the concern about
This would keep the explicit overloads for users who need specific filenames. It also safeguards against future errors by validating the filename-purpose relationship at construction time. Of course, the downside will be the maintenance overhead, and keeping mappings updated as these grow. But with changes to a single mapping class, we can avoid making several update across the repo. Additionally, the maintenance overhead sits with SDK owners, with infrequent updates to If we deprecate single-arg method, every client must update their code. But if we maintain the mapping, only SDK maintainers update It also becomes the single source of truth, with isolated tests, and should be easy to extend. Open to alternatives and suggestions if you disagree. |
Summary
Fixes multipart upload behavior for stream/byte file inputs by ensuring a filename is always present.
This addresses #284, where uploads using anonymous bytes/streams (for example, browser recordings) can fail because multipart
filenameis omitted.Changes
1) Explicit filename overloads
Added filename-aware overloads in:
com.openai.models.files.FileCreateParamscom.openai.models.containers.files.FileCreateParamsNew overloads:
file(InputStream file, String filename)file(byte[] file, String filename)2) Default filename fallback for anonymous uploads
Updated single-argument overloads to provide a safe default filename:
file(InputStream)now defaults tofile.binfile(byte[])now defaults tofile.binThis keeps backward compatibility while improving behavior for anonymous stream/bytes sources.
3) Existing behavior retained
file(Path)behavior is unchanged and continues to usepath.nameas filename.application/octet-stream.4) Tests
Added/updated tests to verify:
file.bin) is applied for single-arg stream/bytes uploadsapplication/octet-streamWhy
file(Path)already included multipart filename metadata, butfile(InputStream)andfile(byte[])previously did not.Some server-side file handling expects
filenamein multipart parts, causing failures for anonymous stream/bytes uploads.Validation
Ran targeted tests for modified models: