Address Common vs REST Defaults for Clients #254
Conversation
|
Warning Rate limit exceeded@dvonthenen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 37 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes streamline and update the functionality across several packages in the codebase. Key modifications include simplifying WebSocket initialization, updating request handling methods, embedding RESTClient types for more consistent API usage, and marking fields as deprecated. These transformations bring more uniformity, improve error handling, and prepare the code for future updates. Changes
Sequence DiagramsWebSocket InitializationsequenceDiagram
participant Main
participant Speak
Note right of Main: Old Flow
Main->>Speak: speak.NewWebSocket(ctx, "", cOptions, ttsOptions, callback)
Note right of Main: New Flow
Main->>Speak: speak.NewWebSocketWithDefaults(ctx, ttsOptions, callback)
Client Setup and Request HandlingsequenceDiagram
participant Client
participant Common
Note right of Client: Old Flow
Client->>Common: c.HTTPClient.Do(ctx, req, func(res *http.Response) error { ... })
Note right of Client: New Flow
Client->>Common: c.Do(ctx, req, resBody)
Deprecated FieldsequenceDiagram
participant Client
Note right of Client: New Flow
Client->>Note: Tier in LiveTranscriptionOptions <br>Deprecated: This field will be <br>removed in a future release.
These diagrams provide a visual guide to key changes in control flow and functional updates in the codebase. They illustrate both old and new implementation flows to highlight differences. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- examples/text-to-speech/websocket/simple/main.go (1 hunks)
- pkg/api/manage/v1/manage.go (1 hunks)
- pkg/api/speak/v1/rest/speak.go (2 hunks)
- pkg/client/analyze/v1/client.go (3 hunks)
- pkg/client/common/v1/common.go (1 hunks)
- pkg/client/common/v1/types.go (1 hunks)
- pkg/client/listen/v1/rest/client.go (1 hunks)
- pkg/client/manage/v1/client.go (1 hunks)
- pkg/client/rest/v1/rest.go (5 hunks)
- pkg/client/speak/v1/rest/client.go (2 hunks)
Additional comments not posted (20)
pkg/client/common/v1/types.go (2)
11-11: LGTM! Introduce the type aliasRESTClient.The introduction of the type alias
RESTClientimproves readability and consistency.
15-15: LGTM! EmbedRESTClientinClient.Embedding
RESTClientinClientaligns with the PR objectives to clarify the hierarchy and usage of various client levels.However, ensure that all instances of
Clientin the codebase are updated to reflect this change.pkg/api/manage/v1/manage.go (1)
32-32: LGTM! Initialize theRESTClientfield.Initializing the
RESTClientfield aligns with the PR objectives to clarify the hierarchy and usage of various client levels.However, ensure that all instances of
Clientinitialization in the codebase are updated to reflect this change.pkg/client/manage/v1/client.go (2)
55-56: LGTM! UseSetupRequestmethod fromRESTClient.The comments clarify the use of
SetupRequestmethod fromRESTClient, aligning with the PR objectives.
63-72: LGTM! UseDomethod fromRESTClient.The comments clarify the use of
Domethod fromRESTClient, aligning with the PR objectives.However, ensure that all instances of
Domethod calls in the codebase are updated to reflect this change.pkg/client/speak/v1/rest/client.go (1)
84-91: LGTM! But verify the function usage in the codebase.The changes are approved. The function now wraps the text in a JSON object and encodes it before sending the request.
However, ensure that all function calls to
DoTextmatch the new signature and usage.pkg/api/speak/v1/rest/speak.go (4)
Line range hint
31-33: LGTM!The changes are approved. The function calls
DoTextwith the appropriate parameters.
Line range hint
52-54: LGTM!The changes are approved. The function calls
DoTextwith the appropriate parameters.
Line range hint
72-74: LGTM!The changes are approved. The function calls
ToFilewith the appropriate parameters.
Line range hint
123-125: LGTM!The changes are approved. The function processes the response and returns the appropriate values.
pkg/client/rest/v1/rest.go (2)
Line range hint
48-71: LGTM!The changes are approved. The function sets the appropriate headers for the request.
Line range hint
74-136: LGTM!The changes are approved. The function handles the response appropriately and processes the result.
pkg/client/common/v1/common.go (3)
Line range hint
48-65: LGTM!The changes are approved. The function sets the appropriate headers for the request.
Line range hint
67-92: LGTM!The changes are approved. The function handles the response appropriately and processes the result.
Line range hint
94-132: LGTM!The changes are approved. The function processes the response body appropriately and returns the result.
pkg/client/listen/v1/rest/client.go (3)
Line range hint
222-223: Verify the usage ofc.SetupRequest.Ensure that
c.SetupRequestis correctly used and that it sets the necessary headers for the request.Verification successful
Verification successful.
The usage of
c.SetupRequestin thepkg/client/listen/v1/rest/client.gofile is correct. The method is implemented in theClientstruct and prepares a new HTTP request with common headers set.
pkg/client/rest/v1/rest.go: func (c *Client) SetupRequest(ctx context.Context, method, uri string, body io.Reader) (*http.Request, error)pkg/client/common/v1/common.go: func (c *Client) SetupRequest(ctx context.Context, method, uri string, body io.Reader) (*http.Request, error)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `SetupRequest` in the `Client`. # Test: Search for the `SetupRequest` method in the `Client`. rg --type go --pattern 'func (c *Client) SetupRequest'Length of output: 468
Script:
#!/bin/bash # Description: Verify the implementation of `SetupRequest` in the `Client`. # Test: Search for the `SetupRequest` method in the `Client`. rg 'func (c \*Client) SetupRequest' --type goLength of output: 47
Script:
#!/bin/bash # Description: Verify the definition or usage of `SetupRequest` across the codebase. # Test: Search for any definition or usage of `SetupRequest`. rg 'SetupRequest' --type goLength of output: 2423
126-129: Verify the usage ofRESTClient.SetupRequest.Ensure that
RESTClient.SetupRequestis correctly used and that it sets the necessary headers for the request.
136-145: Verify the usage ofRESTClient.Do.Ensure that
RESTClient.Dois correctly used and handles the response appropriately.pkg/client/analyze/v1/client.go (2)
222-223: Verify the usage ofc.SetupRequest.Ensure that
c.SetupRequestis correctly used and that it sets the necessary headers for the request.
153-154: Verify the usage ofc.SetupRequest.Ensure that
c.SetupRequestis correctly used and that it sets the necessary headers for the request.Verification successful
Verify the usage of
c.SetupRequest.The
SetupRequestmethod inpkg/client/common/v1/common.gois correctly implemented and sets the necessary headers for the request. The comment about usingc.SetupRequestinstead ofc.RESTClient.SetupRequestis valid.
pkg/client/common/v1/common.go:
func (c *Client) SetupRequest(ctx context.Context, method, uri string, body io.Reader) (*http.Request, error)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `SetupRequest` in the `Client`. # Test: Search for the `SetupRequest` method in the `Client`. rg --type go --pattern 'func (c *Client) SetupRequest'Length of output: 468
Script:
#!/bin/bash # Description: Verify the implementation of `SetupRequest` in the `Client`. # Test: Search for the `SetupRequest` method in the `Client`. rg 'func (c \*Client\) SetupRequest'Length of output: 126
Script:
#!/bin/bash # Description: Verify the implementation of `SetupRequest` in the `Client`. # Test: Search for the `SetupRequest` method in the `Client`. rg 'func \(c \*Client\) SetupRequest'Length of output: 321
Script:
#!/bin/bash # Description: Check the imports in `pkg/client/analyze/v1/client.go` to determine which `SetupRequest` method is being used. # Test: Search for import statements in `pkg/client/analyze/v1/client.go`. rg '^import' -A 10 pkg/client/analyze/v1/client.goLength of output: 155
Script:
#!/bin/bash # Description: Search for `SetupRequest` references within `pkg/client/analyze/v1/client.go` and trace its origin. # Test: Search for `SetupRequest` references in `pkg/client/analyze/v1/client.go`. rg 'SetupRequest' pkg/client/analyze/v1/client.go -A 5 -B 5Length of output: 1163
Script:
#!/bin/bash # Description: Search for the declaration of `c` within `pkg/client/analyze/v1/client.go` to determine its type. # Test: Search for `c :=` or `var c` in `pkg/client/analyze/v1/client.go`. rg '\bc\b' pkg/client/analyze/v1/client.go -A 5 -B 5Length of output: 4552
Script:
#!/bin/bash # Description: Verify the `common.New` function to ensure it initializes `SetupRequest` correctly. # Test: Locate and inspect the `New` function in `pkg/client/common/v1/common.go`. rg 'func New' pkg/client/common/v1/common.go -A 10 -B 10Length of output: 515
Script:
#!/bin/bash # Description: Retrieve the complete implementation of the `New` function in `pkg/client/common/v1/common.go`. # Test: Display the complete implementation of the `New` function. rg 'func New' pkg/client/common/v1/common.go -A 50Length of output: 1664
dvonthenen
force-pushed
the
fix-common-rest-client-use
branch
from
aaf7892
to
94d3475
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- examples/text-to-speech/websocket/simple/main.go (1 hunks)
- pkg/api/manage/v1/manage.go (1 hunks)
- pkg/api/speak/v1/rest/speak.go (2 hunks)
- pkg/client/analyze/v1/client.go (3 hunks)
- pkg/client/common/v1/common.go (1 hunks)
- pkg/client/common/v1/types.go (1 hunks)
- pkg/client/interfaces/v1/types-stream.go (1 hunks)
- pkg/client/listen/v1/rest/client.go (1 hunks)
- pkg/client/manage/v1/client.go (1 hunks)
- pkg/client/rest/v1/rest.go (5 hunks)
- pkg/client/speak/v1/rest/client.go (2 hunks)
Files skipped from review as they are similar to previous changes (9)
- examples/text-to-speech/websocket/simple/main.go
- pkg/api/manage/v1/manage.go
- pkg/api/speak/v1/rest/speak.go
- pkg/client/analyze/v1/client.go
- pkg/client/common/v1/common.go
- pkg/client/common/v1/types.go
- pkg/client/listen/v1/rest/client.go
- pkg/client/manage/v1/client.go
- pkg/client/rest/v1/rest.go
Additional comments not posted (4)
pkg/client/interfaces/v1/types-stream.go (1)
40-40: Deprecation Notice AddedThe
Tierfield has been marked as deprecated. Ensure that any usage of this field is updated or removed in future releases.pkg/client/speak/v1/rest/client.go (3)
84-90: LGTM! JSON Encoding for Text SourceThe text is now properly encoded as JSON before being sent in the request. This ensures that the text is correctly formatted for the API.
92-93: LGTM! HTTP Request SetupThe
SetupRequestmethod is used to create the HTTP request, ensuring consistent request setup with common headers.
103-111: LGTM! Handling of Response HeadersThe use of
HTTPClient.DoandHandleResponsemethods allows for the extraction of response headers, which is necessary for processing the response correctly.
dvonthenen
force-pushed
the
fix-common-rest-client-use
branch
2 times, most recently
from
f9a480e
to
4ad60ed
Compare
dvonthenen
force-pushed
the
fix-common-rest-client-use
branch
from
4ad60ed
to
f038bf5
Compare
Proposed changes
This addresses which internal client to use in which situation. It turned out that the Common Client was being used in just about every situation and that the REST client, which is a lower-level client (although not being used), had a more restricted options set.
This clears up the confusion in the SDK and also makes the various level of client (API specific [most specific] - common - rest - HTTP [most general]). These are all internal changes not exposed to the user.
Tested all examples and they seem to work just fine.
Types of changes
What types of changes does your code introduce to the community Go SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
NA
Summary by CodeRabbit
Refactor
Clientstructs to useRESTClientfor more consistent request handling across the system.Bug Fixes
performActionfunction to clean up console output.Tierfield inLiveTranscriptionOptions, preparing for future removal.Enhancements