feat(examples): add OpenAI-compatible benchmark sample - #686
Open
FamousDirector wants to merge 6 commits into
Open
feat(examples): add OpenAI-compatible benchmark sample#686FamousDirector wants to merge 6 commits into
FamousDirector wants to merge 6 commits into
Conversation
Add controllable OpenAI endpoints for SDK and load testing. NO-REF Signed-off-by: jcameron <jcameron@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an OpenAI-compatible Go HTTP server with JSON and SSE endpoints, validation tests, compatibility checks, Docker packaging, documentation, and a k6 Responses SSE load test. ChangesOpenAI-compatible sample
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPServer
participant K6LoadTest
participant OpenAIClient
Client->>HTTPServer: Send OpenAI-compatible request
OpenAIClient->>HTTPServer: Run compatibility checks
K6LoadTest->>HTTPServer: Send Responses SSE load
HTTPServer-->>Client: Return JSON or SSE events
HTTPServer-->>OpenAIClient: Return validated API response
HTTPServer-->>K6LoadTest: Stream response events and timing data
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
examples/function-samples/openai-compatible-sample/http-server/main_test.go (1)
245-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset the shared gauge in cleanup.
activeRequestsis package-level state shared by every test. This test resets it only at the start. If an assertion fails while the first request still holds a slot, the gauge stays non-zero for later tests. Add a cleanup reset.Proposed refactor
activeRequests.Store(0) + t.Cleanup(func() { activeRequests.Store(0) }) router := newRouter()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go` around lines 245 - 247, Update TestConcurrencyLimit to register a cleanup reset for the shared activeRequests gauge, ensuring it is restored to zero even when the test fails before its normal completion. Keep the existing initial reset and use the test cleanup mechanism so later tests cannot inherit stale state.examples/function-samples/openai-compatible-sample/http-server/main.go (3)
880-883: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the SSE header setup.
The same three-header block appears in
streamResponses(lines 881-883),streamChatCompletion(lines 964-966), andstreamCompletion(lines 1045-1047). Extract one helper so SSE header changes apply in one place.Also note that Go's HTTP server manages
Connectionitself and strips it for HTTP/2, so that header has no effect. Consider dropping it from the helper.Proposed refactor
+func setSSEHeaders(w http.ResponseWriter) { + w.Header().Set("Cache-Control", "no-cache") + w.Header().Set("Content-Type", "text/event-stream") +} + func streamResponses(ctx context.Context, w http.ResponseWriter, response responsesResponse, chunks []string, tuning benchmarkTuning) { - w.Header().Set("Cache-Control", "no-cache") - w.Header().Set("Content-Type", "text/event-stream") - w.Header().Set("Connection", "keep-alive") + setSSEHeaders(w)Apply the same replacement in
streamChatCompletionandstreamCompletion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/function-samples/openai-compatible-sample/http-server/main.go` around lines 880 - 883, Extract the repeated SSE header setup from streamResponses, streamChatCompletion, and streamCompletion into a shared helper so all three call the same header-setting logic. Update that helper to set the common SSE headers in one place and remove the Connection header from the shared setup, since net/http manages it and it has no effect here. Keep the existing streaming behavior unchanged aside from centralizing the header configuration.
655-665: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused error return from
outputChunks.
outputChunksnever returns a non-nil error. Validation already happens inresolveBenchmarkTuning. The current signature creates three unreachable 400 branches inhandleResponses,handleChatCompletions, andhandleCompletions.Proposed refactor
-func outputChunks(tuning benchmarkTuning) ([]string, error) { +func outputChunks(tuning benchmarkTuning) []string { chunks := make([]string, tuning.OutputChunks) for index := range chunks { chunk := tuning.Chunk if tuning.ChunkBytes > 0 { chunk = randomText(tuning.ChunkBytes) } chunks[index] = chunk } - return chunks, nil + return chunks }Then in each handler:
chunks := outputChunks(tuning)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/function-samples/openai-compatible-sample/http-server/main.go` around lines 655 - 665, Update outputChunks to return only []string, removing its unused error result and adjusting the return statement. Update handleResponses, handleChatCompletions, and handleCompletions to call outputChunks without error handling, removing the unreachable 400-response branches while preserving their existing chunk-processing behavior.
272-280: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound body reads and idle connections.
The server sets only
ReadHeaderTimeout. A client that sends headers and then stalls the body, or that keeps an idle connection open, retains a connection and a goroutine without limit. AddReadTimeoutandIdleTimeout.Do not add
WriteTimeout. SSE responses can run for minutes becauseX-Load-Tester-ITL-MsandX-Load-Tester-Output-Chunkscontrol the stream duration, andWriteTimeoutwould abort valid streams.Proposed fix
server := &http.Server{ Addr: ":8000", Handler: newRouter(), ReadHeaderTimeout: 5 * time.Second, + ReadTimeout: 30 * time.Second, + IdleTimeout: 120 * time.Second, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/function-samples/openai-compatible-sample/http-server/main.go` around lines 272 - 280, Update the http.Server initialization in main to add ReadTimeout for bounding request body reads and IdleTimeout for limiting keep-alive connections. Preserve ReadHeaderTimeout and do not add WriteTimeout, so long-running SSE responses controlled by X-Load-Tester-ITL-Ms and X-Load-Tester-Output-Chunks continue uninterrupted.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/function-samples/load-tester-supreme/README.md`:
- Line 22: Update the message field documentation in the load-tester README to
specify protocol-specific requirements: require message for HTTP requests, while
allowing it to be omitted for gRPC and documenting that omission returns an
empty string.
In `@examples/function-samples/openai-compatible-sample/Dockerfile`:
- Around line 24-30: Update the final alpine stage after copying the binary to
create an unprivileged user named app, assign ownership of
/app/openai-compatible-sample to that user, and set USER app before CMD so the
server runs without root privileges.
In
`@examples/function-samples/openai-compatible-sample/http-server/openai_client_check.py`:
- Line 41: Replace every assertion in openai_client_check.py, including the
checks around response.output_text and the other listed validation points, with
explicit conditional checks that raise an error when expectations fail. Ensure
all compatibility checks still execute under python3 -O and success is printed
only after every condition passes.
- Around line 24-28: Update the OpenAI client initialization in the CLIENT
configuration to disable redirects for credentialed requests by supplying an
httpx.Client with follow_redirects set to false through http_client, while
preserving the existing base URL and authentication settings.
- Around line 24-28: In the OpenAI client initialization around BASE_URL and
OPENAI_API_KEY, validate that any non-placeholder API key is only used with an
HTTPS base URL. Preserve the local HTTP URL for the default "not-needed" key,
and reject or fail fast when a real key is configured with an HTTP URL before
constructing CLIENT.
In `@examples/load-tests/functions/oai_compatible_responses_sse_load_test.js`:
- Around line 219-224: Remove the unsupported timeout configuration from the
load-test options and eliminate the related OPENAI_RESPONSES_TIMEOUT
environment-variable usage, unless upgrading xk6-sse and implementing verified
timeout behavior through the sse.open call. Ensure the stream setup does not
pass a silently ignored timeout value.
- Around line 283-296: Update the request configuration returned by the OpenAI
Responses SSE load-test request builder to reject redirects before sending
credentialed requests with TOKEN. Since xk6-sse v0.1.7 does not expose a
redirects option, apply the required client/extension configuration rather than
adding an unsupported request field, while preserving the existing POST body,
headers, timeout, and tags.
- Around line 272-274: Update the header setup around config.token so the
OAI_COMPAT_URL is validated before assigning headers.Authorization. Reject HTTP
endpoints unless they target an explicit loopback host, while allowing HTTPS
endpoints, and only add the Bearer token after this validation succeeds.
---
Nitpick comments:
In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go`:
- Around line 245-247: Update TestConcurrencyLimit to register a cleanup reset
for the shared activeRequests gauge, ensuring it is restored to zero even when
the test fails before its normal completion. Keep the existing initial reset and
use the test cleanup mechanism so later tests cannot inherit stale state.
In `@examples/function-samples/openai-compatible-sample/http-server/main.go`:
- Around line 880-883: Extract the repeated SSE header setup from
streamResponses, streamChatCompletion, and streamCompletion into a shared helper
so all three call the same header-setting logic. Update that helper to set the
common SSE headers in one place and remove the Connection header from the shared
setup, since net/http manages it and it has no effect here. Keep the existing
streaming behavior unchanged aside from centralizing the header configuration.
- Around line 655-665: Update outputChunks to return only []string, removing its
unused error result and adjusting the return statement. Update handleResponses,
handleChatCompletions, and handleCompletions to call outputChunks without error
handling, removing the unreachable 400-response branches while preserving their
existing chunk-processing behavior.
- Around line 272-280: Update the http.Server initialization in main to add
ReadTimeout for bounding request body reads and IdleTimeout for limiting
keep-alive connections. Preserve ReadHeaderTimeout and do not add WriteTimeout,
so long-running SSE responses controlled by X-Load-Tester-ITL-Ms and
X-Load-Tester-Output-Chunks continue uninterrupted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 007b31ca-4460-4a32-9404-6b5c5c9e701f
📒 Files selected for processing (11)
examples/README.mdexamples/function-samples/load-tester-supreme/Dockerfileexamples/function-samples/load-tester-supreme/README.mdexamples/function-samples/openai-compatible-sample/Dockerfileexamples/function-samples/openai-compatible-sample/README.mdexamples/function-samples/openai-compatible-sample/http-server/go.modexamples/function-samples/openai-compatible-sample/http-server/main.goexamples/function-samples/openai-compatible-sample/http-server/main_test.goexamples/function-samples/openai-compatible-sample/http-server/openai_client_check.pyexamples/load-tests/README.mdexamples/load-tests/functions/oai_compatible_responses_sse_load_test.js
Signed-off-by: jcameron <jcameron@nvidia.com>
Reduce allocation and write overhead for long-lived OpenAI-compatible SSE streams while retaining compatible event and timing behavior. NO-REF Signed-off-by: jcameron <jcameron@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/function-samples/openai-compatible-sample/http-server/main_test.go (1)
470-478: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive assertion for the truncated Responses stream.
The block asserts only the absence of
event: errorandevent: response.completed. An empty body, or a body truncated before the first delta, also satisfies both conditions. Assert the delta count that truncation is expected to emit, so the test detects a regression that stops the stream too early.♻️ Proposed assertion
body = recorder.Body.String() if strings.Contains(body, "event: error") || strings.Contains(body, "event: response.completed") { t.Fatalf("truncated Responses stream unexpectedly terminated: %s", body) } + if got := strings.Count(body, "event: response.output_text.delta\n"); got != 1 { + t.Fatalf("truncated Responses deltas = %d, want 1: %s", got, body) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go` around lines 470 - 478, The truncated Responses stream test around postJSONWithHeaders must positively verify that the expected number of delta events was emitted, not only that terminal events are absent. Add an assertion using the stream’s existing delta event marker to confirm truncation after the configured boundary produces the expected delta count, while preserving the current error and response.completed checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go`:
- Around line 470-478: The truncated Responses stream test around
postJSONWithHeaders must positively verify that the expected number of delta
events was emitted, not only that terminal events are absent. Add an assertion
using the stream’s existing delta event marker to confirm truncation after the
configured boundary produces the expected delta count, while preserving the
current error and response.completed checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1f921b5c-74e8-4112-8897-58d43ed2d517
📒 Files selected for processing (4)
examples/function-samples/openai-compatible-sample/README.mdexamples/function-samples/openai-compatible-sample/http-server/main.goexamples/function-samples/openai-compatible-sample/http-server/main_test.goexamples/load-tests/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/load-tests/README.md
- examples/function-samples/openai-compatible-sample/http-server/main.go
- examples/function-samples/openai-compatible-sample/README.md
NO-REF Signed-off-by: jcameron <jcameron@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go`:
- Line 350: Replace httptest.NewRequest with httptest.NewRequestWithContext at
both request construction sites in
examples/function-samples/openai-compatible-sample/http-server/main_test.go
lines 350-350 and 389-389, supplying an explicit context while preserving the
existing method, URL, and body arguments.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b89b433-911d-4a83-8377-8a3623777598
📒 Files selected for processing (4)
examples/function-samples/openai-compatible-sample/README.mdexamples/function-samples/openai-compatible-sample/http-server/main.goexamples/function-samples/openai-compatible-sample/http-server/main_test.goexamples/load-tests/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/function-samples/openai-compatible-sample/README.md
- examples/function-samples/openai-compatible-sample/http-server/main.go
- examples/load-tests/README.md
| {name: "above default maximum rejected", value: "6001", want: false}, | ||
| } { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| request := httptest.NewRequest(http.MethodPost, "/v1/responses", nil) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use context-aware request construction.
golangci-lint reports noctx errors at both sites. Replace httptest.NewRequest with httptest.NewRequestWithContext and provide an explicit context.
examples/function-samples/openai-compatible-sample/http-server/main_test.go#L350-L350: Construct the request withhttptest.NewRequestWithContext.examples/function-samples/openai-compatible-sample/http-server/main_test.go#L389-L389: Construct the request withhttptest.NewRequestWithContext.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 350-350: net/http/httptest.NewRequest must not be called. use net/http/httptest.NewRequestWithContext
(noctx)
📍 Affects 1 file
examples/function-samples/openai-compatible-sample/http-server/main_test.go#L350-L350(this comment)examples/function-samples/openai-compatible-sample/http-server/main_test.go#L389-L389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/function-samples/openai-compatible-sample/http-server/main_test.go`
at line 350, Replace httptest.NewRequest with httptest.NewRequestWithContext at
both request construction sites in
examples/function-samples/openai-compatible-sample/http-server/main_test.go
lines 350-350 and 389-389, supplying an explicit context while preserving the
existing method, URL, and body arguments.
Source: Linters/SAST tools
Signed-off-by: jcameron <jcameron@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/function-samples/openai-compatible-sample/http-server/main.go`:
- Around line 630-653: Remove the redundant initial assignments to
chunkBytesName, statusName, streamErrorName, and streamTruncateName in the
benchmark tuning logic; declare these variables directly through the
corresponding integerControl assignments while preserving the existing
returned-name behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7180828f-8e6e-448f-b782-2f2717412460
📒 Files selected for processing (3)
examples/function-samples/openai-compatible-sample/README.mdexamples/function-samples/openai-compatible-sample/http-server/main.goexamples/function-samples/openai-compatible-sample/http-server/main_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/function-samples/openai-compatible-sample/README.md
- examples/function-samples/openai-compatible-sample/http-server/main_test.go
Comment on lines
+630
to
+653
| chunkBytesName := headerChunkBytes | ||
| var hasChunkBytes bool | ||
| if tuning.ChunkBytes, hasChunkBytes, chunkBytesName, err = integerControl(r, headerChunkBytes, bodyChunkBytes, body.ChunkBytes, 0, 0, maxOutputBytes); err != nil { | ||
| return benchmarkTuning{}, err | ||
| } | ||
| if hasChunkBytes && tuning.ChunkBytes > 0 && hasChunk { | ||
| return benchmarkTuning{}, fmt.Errorf("%s and %s cannot be combined", chunkName, chunkBytesName) | ||
| } | ||
| if tuning.OutputChunks, _, _, err = integerControl(r, headerOutputChunks, bodyOutputChunks, body.OutputChunks, 1, 1, maxOutputChunks); err != nil { | ||
| return benchmarkTuning{}, err | ||
| } | ||
| statusName := headerStatusCode | ||
| if tuning.StatusCode, _, statusName, err = integerControl(r, headerStatusCode, bodyStatusCode, body.StatusCode, 0, 0, 599); err != nil { | ||
| return benchmarkTuning{}, err | ||
| } | ||
| if tuning.StatusCode != 0 && tuning.StatusCode < http.StatusBadRequest { | ||
| return benchmarkTuning{}, fmt.Errorf("%s must be an HTTP error status", statusName) | ||
| } | ||
| streamErrorName := headerStreamErrorAfter | ||
| if tuning.StreamErrorAfter, _, streamErrorName, err = integerControl(r, headerStreamErrorAfter, bodyStreamErrorAfter, body.StreamErrorAfter, -1, -1, tuning.OutputChunks); err != nil { | ||
| return benchmarkTuning{}, err | ||
| } | ||
| streamTruncateName := headerStreamTruncateAfter | ||
| if tuning.StreamTruncateAfter, _, streamTruncateName, err = integerControl(r, headerStreamTruncateAfter, bodyStreamTruncateAfter, body.StreamTruncateAfter, -1, -1, tuning.OutputChunks); err != nil { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the ineffectual initial assignments.
Line 630 assigns headerChunkBytes, but Line 632 overwrites chunkBytesName before use. The same issue occurs for statusName, streamErrorName, and streamTruncateName at Lines 641, 648, and 652. golangci-lint reports these as errors.
Proposed fix
- chunkBytesName := headerChunkBytes
+ var chunkBytesName string
var hasChunkBytes bool
if tuning.ChunkBytes, hasChunkBytes, chunkBytesName, err = integerControl(r, headerChunkBytes, bodyChunkBytes, body.ChunkBytes, 0, 0, maxOutputBytes); err != nil {
return benchmarkTuning{}, err
}
@@
- statusName := headerStatusCode
+ var statusName string
if tuning.StatusCode, _, statusName, err = integerControl(r, headerStatusCode, bodyStatusCode, body.StatusCode, 0, 0, 599); err != nil {
return benchmarkTuning{}, err
}
@@
- streamErrorName := headerStreamErrorAfter
+ var streamErrorName string
if tuning.StreamErrorAfter, _, streamErrorName, err = integerControl(r, headerStreamErrorAfter, bodyStreamErrorAfter, body.StreamErrorAfter, -1, -1, tuning.OutputChunks); err != nil {
return benchmarkTuning{}, err
}
- streamTruncateName := headerStreamTruncateAfter
+ var streamTruncateName string
if tuning.StreamTruncateAfter, _, streamTruncateName, err = integerControl(r, headerStreamTruncateAfter, bodyStreamTruncateAfter, body.StreamTruncateAfter, -1, -1, tuning.OutputChunks); err != nil {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| chunkBytesName := headerChunkBytes | |
| var hasChunkBytes bool | |
| if tuning.ChunkBytes, hasChunkBytes, chunkBytesName, err = integerControl(r, headerChunkBytes, bodyChunkBytes, body.ChunkBytes, 0, 0, maxOutputBytes); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| if hasChunkBytes && tuning.ChunkBytes > 0 && hasChunk { | |
| return benchmarkTuning{}, fmt.Errorf("%s and %s cannot be combined", chunkName, chunkBytesName) | |
| } | |
| if tuning.OutputChunks, _, _, err = integerControl(r, headerOutputChunks, bodyOutputChunks, body.OutputChunks, 1, 1, maxOutputChunks); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| statusName := headerStatusCode | |
| if tuning.StatusCode, _, statusName, err = integerControl(r, headerStatusCode, bodyStatusCode, body.StatusCode, 0, 0, 599); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| if tuning.StatusCode != 0 && tuning.StatusCode < http.StatusBadRequest { | |
| return benchmarkTuning{}, fmt.Errorf("%s must be an HTTP error status", statusName) | |
| } | |
| streamErrorName := headerStreamErrorAfter | |
| if tuning.StreamErrorAfter, _, streamErrorName, err = integerControl(r, headerStreamErrorAfter, bodyStreamErrorAfter, body.StreamErrorAfter, -1, -1, tuning.OutputChunks); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| streamTruncateName := headerStreamTruncateAfter | |
| if tuning.StreamTruncateAfter, _, streamTruncateName, err = integerControl(r, headerStreamTruncateAfter, bodyStreamTruncateAfter, body.StreamTruncateAfter, -1, -1, tuning.OutputChunks); err != nil { | |
| var chunkBytesName string | |
| var hasChunkBytes bool | |
| if tuning.ChunkBytes, hasChunkBytes, chunkBytesName, err = integerControl(r, headerChunkBytes, bodyChunkBytes, body.ChunkBytes, 0, 0, maxOutputBytes); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| if hasChunkBytes && tuning.ChunkBytes > 0 && hasChunk { | |
| return benchmarkTuning{}, fmt.Errorf("%s and %s cannot be combined", chunkName, chunkBytesName) | |
| } | |
| if tuning.OutputChunks, _, _, err = integerControl(r, headerOutputChunks, bodyOutputChunks, body.OutputChunks, 1, 1, maxOutputChunks); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| var statusName string | |
| if tuning.StatusCode, _, statusName, err = integerControl(r, headerStatusCode, bodyStatusCode, body.StatusCode, 0, 0, 599); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| if tuning.StatusCode != 0 && tuning.StatusCode < http.StatusBadRequest { | |
| return benchmarkTuning{}, fmt.Errorf("%s must be an HTTP error status", statusName) | |
| } | |
| var streamErrorName string | |
| if tuning.StreamErrorAfter, _, streamErrorName, err = integerControl(r, headerStreamErrorAfter, bodyStreamErrorAfter, body.StreamErrorAfter, -1, -1, tuning.OutputChunks); err != nil { | |
| return benchmarkTuning{}, err | |
| } | |
| var streamTruncateName string | |
| if tuning.StreamTruncateAfter, _, streamTruncateName, err = integerControl(r, headerStreamTruncateAfter, bodyStreamTruncateAfter, body.StreamTruncateAfter, -1, -1, tuning.OutputChunks); err != nil { |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 630-630: ineffectual assignment to chunkBytesName
(ineffassign)
[error] 641-641: ineffectual assignment to statusName
(ineffassign)
[error] 648-648: ineffectual assignment to streamErrorName
(ineffassign)
[error] 652-652: ineffectual assignment to streamTruncateName
(ineffassign)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/function-samples/openai-compatible-sample/http-server/main.go`
around lines 630 - 653, Remove the redundant initial assignments to
chunkBytesName, statusName, streamErrorName, and streamTruncateName in the
benchmark tuning logic; declare these variables directly through the
corresponding integerControl assignments while preserving the existing
returned-name behavior.
Source: Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Add a controllable OpenAI-compatible function sample for SDK and load-test benchmarking.
Additional Details
For the Reviewer
Review benchmark header validation and stream termination behavior in the sample server. The client check exercises strict response validation through the public OpenAI Python SDK.
For QA
QA Needed: No. Sample-only change with automated and local SDK coverage.
Issues
NO-REF
Checklist
Summary by CodeRabbit