Repository navigation
feat(core): expose request body content streams - #923
sylvesterkaczmarek wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d392208bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3d39220 to
dc2f9cd
Compare
|
I resolved this PR against current main while preserving the intended fix and newer upstream behavior. The branch is now mergeable and no longer has a base conflict. Could a maintainer please re-review it when convenient? @codex review |
|
Addressed the DEBUG logging streaming regression in f5997f3. LoggingHttpRequestBody now implements a streaming content() wrapper that logs incrementally instead of falling back to the buffering default, with a regression test whose delegate consumes content() and fails if writeTo() is used. The review thread is resolved. @codex review |
Resolved against current multipart lifecycle handling.
f5997f3 to
7721924
Compare
|
Thank you, Sylvester, for the request-body streaming proposal, tests, and logging fix. We built on this work in #1245, which has now merged and adds HttpRequestBody.content(). Closing this PR as superseded by that implementation. |
Custom `HttpClient` adapters currently rebuild header maps and bridge `HttpRequestBody.writeTo(OutputStream)` into an input stream. Add the two accessors discussed in [openai-java#664](openai#664): ```java Map<String, List<String>> headers = request.headers().asMap(); try (InputStream content = body.content()) { // Give content to the transport and wait for consumption to finish. } // Close body after the final attempt, including failure or cancellation. ``` `Headers.asMap()` exposes an immutable, case-insensitive view with immutable value lists. `HttpRequestBody.content()` has a default implementation for existing Java implementations; that fallback buffers `writeTo()` output. Built-in JSON bodies reuse their encoded bytes, while multipart bodies stream raw uploads lazily. Repeatability, close-once upload cleanup, and explicitly enabled DEBUG logging are preserved. `HttpResponse.body()` already returns an `InputStream`. Includes coverage for header immutability, Java source/binary compatibility, multipart byte equivalence, shared uploads, early close and cleanup failures, cancellation, retries, and stream-based request logging. Builds on Sylvester Kaczmarek's [headers proposal openai#900](openai#900) and [request-stream proposal openai#923](openai#923). Validation: 296 selected tests passed; no failures or skips. Kotlin lint passed. ```sh SKIP_MOCK_TESTS=true ./gradlew :openai-java-core:test \ --tests '*HttpRequestBodyContentTest' --tests '*HttpRequestBodyJavaTest' \ --tests '*HeadersMapViewTest' --tests '*LoggingHttpClientTest' \ --tests '*HeadersTest' --tests '*HttpRequestBodiesTest' \ --tests '*MultipartBodyCleanupTest' --tests '*RetryingHttpClientTest' \ --tests '*UnionUploadStreaming*' :openai-java-core:lintKotlin ``` Co-authored-by: dpiet-oai <326640584+dpiet-oai@users.noreply.github.com>
Summary
Add
HttpRequestBody.content(): InputStreamso custom HTTP client implementations can consume request bodies directly as streams.Addresses the request-body portion of #664.
Problem
HttpResponse.body()already exposes anInputStream, butHttpRequestBodyonly exposeswriteTo(OutputStream). CustomHttpClientadapters therefore have to bridge the push-style request body API themselves.Changes
content(): InputStreammethod toHttpRequestBodywriteToSequenceInputStream, soInputStream-backed file parts are not buffered into memoryCompatibility
The repository compiles Kotlin interfaces with
-Xjvm-default=all, so the new method is emitted as a Java default interface method. Existing Java and Kotlin implementations are not required to add an override.The fallback implementation buffers only for third-party bodies that do not override
content(). Built-in SDK request bodies override it, including streaming multipart bodies.Validation
mainat715a30468d3232a5df4f6c6bec8b2175404eb8b9HttpRequestBody, built-in request-body implementations, and focused testsAddresses #664