fix: lint fixes, sync with generator, use common streaming interface for streamed list objects#259
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR applies formatting and whitespace refinements to the API client, augments documentation comments in the OpenFGA interface, and modifies error handling in streaming response processing to capture and propagate HTTP response body close errors through a channel rather than silently ignoring them. Test files are updated to explicitly suppress write error returns. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ProcessStreamedListObjectsResponse
participant HTTPResponse
participant Channel
Caller->>ProcessStreamedListObjectsResponse: Call function
ProcessStreamedListObjectsResponse->>HTTPResponse: Begin reading body
note over ProcessStreamedListObjectsResponse: Defer: Close body & handle error
loop Process stream
ProcessStreamedListObjectsResponse->>HTTPResponse: Read line
HTTPResponse-->>ProcessStreamedListObjectsResponse: Data/EOF
ProcessStreamedListObjectsResponse->>Channel: Send result
end
ProcessStreamedListObjectsResponse->>HTTPResponse: Close body (deferred)
alt Close succeeds
HTTPResponse-->>ProcessStreamedListObjectsResponse: nil
else Close fails
HTTPResponse-->>ProcessStreamedListObjectsResponse: error
ProcessStreamedListObjectsResponse->>Channel: Send error
end
Channel-->>Caller: Results + potential close error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #259 +/- ##
==========================================
+ Coverage 33.82% 33.89% +0.07%
==========================================
Files 114 114
Lines 10990 11022 +32
==========================================
+ Hits 3717 3736 +19
- Misses 6911 6922 +11
- Partials 362 364 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR applies lint fixes and syncs code formatting with the OpenAPI generator for streamed list objects functionality.
Key changes:
- Adds proper error handling for deferred
Body.Close()in streaming response processing - Fixes Go linter warnings by handling return values from
w.Write()calls in test files - Normalizes code indentation from spaces to tabs across multiple files for consistency
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| streaming.go | Added error handling for deferred Body.Close() in streaming response processor |
| streaming_test.go | Fixed linter warnings by capturing w.Write() return values in test handlers |
| client/streaming_test.go | Fixed linter warnings by capturing w.Write() return values in test handlers |
| api_open_fga.go | Normalized indentation from spaces to tabs in API documentation comments |
| api_client.go | Normalized indentation from spaces to tabs throughout the file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
api_client.go(1 hunks)api_open_fga.go(13 hunks)client/streaming_test.go(5 hunks)streaming.go(1 hunks)streaming_test.go(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
api_client.go (9)
configuration.go (2)
Configuration(22-37)RetryParams(19-19)telemetry/configuration.go (2)
Configuration(32-34)DefaultTelemetryConfiguration(36-83)internal/utils/retryutils/retryparams.go (2)
RetryParams(6-9)GetRetryParamsOrDefault(29-35)telemetry/telemetry.go (4)
Telemetry(16-19)Bind(87-89)Get(65-85)TelemetryFactoryParameters(21-23)credentials/credentials.go (1)
Credentials(38-42)errors.go (5)
NewFgaApiValidationError(335-365)NewFgaApiNotFoundError(451-481)NewFgaApiRateLimitExceededError(755-809)NewFgaApiInternalError(593-629)NewFgaApiError(219-249)model_validation_error_message_response.go (1)
ValidationErrorMessageResponse(22-25)model_path_unknown_error_message_response.go (1)
PathUnknownErrorMessageResponse(22-25)model_internal_error_message_response.go (1)
InternalErrorMessageResponse(22-25)
🪛 Gitleaks (8.29.0)
api_open_fga.go
[high] 553-553: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 581-581: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 615-615: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 723-723: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
client/streaming_test.go (1)
43-43: LGTM - Explicit error suppression in tests.Using
_, _ = w.Write()appropriately documents that write errors are intentionally ignored in these test HTTP handlers. This is a common and acceptable pattern in Go tests withhttptest.ResponseWriter.Also applies to: 101-101, 149-149, 216-216, 283-283
api_open_fga.go (1)
42-912: LGTM - Documentation improvements.All changes are documentation-only refinements (formatting, expanded examples, added reference sections). No functional code changes.
streaming_test.go (1)
60-60: LGTM - Explicit error suppression in tests.Using
_, _ = w.Write()appropriately documents that write errors are intentionally ignored in test HTTP handlers, consistent with standard Go test patterns.Also applies to: 118-118, 163-163, 217-217, 265-265, 329-329, 389-389, 440-440
api_client.go (1)
4-499: LGTM - Formatting refinements.All changes are formatting and whitespace adjustments with no behavioral modifications. These align with standard Go formatting conventions.
ewanharris
left a comment
There was a problem hiding this comment.
Fine for separate PR, but maybe we could add a lint/format check within the existing workflow? I guess we never really had it til now because everything would always be linted/formatted by the sdk-generator process
Sure, I'll add a follow up PR immediately after this |
Description
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Bug Fixes
Documentation
Style
Tests