fix(logs): harden task streaming and SSE handling - #318
Serta莽 脰zercan (sozercan) wants to merge 6 commits into
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4284a87401
鈩癸笍 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 (@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 (@codex) address that feedback".
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d408ee1d3
鈩癸笍 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 (@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 (@codex) address that feedback".
| if transport, ok := http.DefaultTransport.(*http.Transport); ok { | ||
| clone := transport.Clone() | ||
| clone.ResponseHeaderTimeout = responseHeaderTimeout | ||
| return &http.Client{Transport: clone} |
There was a problem hiding this comment.
When an embedding program configures http.DefaultClient.Transport鈥攁 supported pattern for mTLS, Kubernetes auth, proxies, or tracing鈥攖hese constructors previously honored it, but now they consult only http.DefaultTransport and silently use an unrelated clone. Consequently every API request can bypass the caller's required authentication or routing. The fresh evidence after the earlier review is that the new fallback preserves a customized http.DefaultTransport, but the constructor still never reads http.DefaultClient.Transport.
Useful? React with 馃憤聽/ 馃憥.
| case <-ctx.Done(): | ||
| return nil |
There was a problem hiding this comment.
When the inherited request context has a deadline that expires after the Kubernetes stream is established, this branch closes the SSE response without emitting an error. StreamTaskLogs interprets that clean EOF as success, so orka task logs --follow silently exits while the task may still be running; distinguish DeadlineExceeded here and send a terminal error event before closing.
Useful? React with 馃憤聽/ 馃憥.
1 participant
Summary
Stacked salvage from #280. Base: #314 (
fix/kubernetes-inventory-pagination).Scope
This PR changes task-log follow streaming and the CLI SSE client only. It does not change backend inventory pagination; the stack base is used to keep the review diff to one commit.
Verification
go test ./internal/api ./internal/cli/client ./cmd/cli -run 'TaskLog|SSE|StreamTaskLogs' -count=1make lint-fix && make testAfter #314 merges, this PR should be retargeted to
main.