refactor(cody): Reshape the CompletionsClient interface#63358
Conversation
| ) (*types.CompletionResponse, error) { | ||
| request types.CompletionRequest) (*types.CompletionResponse, error) { | ||
|
|
||
| feature := request.Feature |
There was a problem hiding this comment.
If we want to do a more invasive refactoring, we can probably just remove these local params and instead have request.Feature be used in-line. But doing this "explode the type into local variables" lead to the smallest diff.
|
Gonna go ahead and merge because I know someone else is working on this same code, to spare some conflicts. |
|
The backport to To backport this PR manually, you can either: Via the sg toolUse the sg backport -r 5.4.5099 -p 63358Via your terminalTo backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.4.5099 5.4.5099
# Navigate to the new working tree
cd .worktrees/backport-5.4.5099
# Create a new branch
git switch --create backport-63358-to-5.4.5099
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 46134524c79dd2b30b619b17d3a30ac61aeb9295
# Push it to GitHub
git push --set-upstream origin backport-63358-to-5.4.5099
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.4.5099If you encouter conflict, first resolve the conflict and stage all files, then run the commands below: git cherry-pick --continue
# Push it to GitHub
git push --set-upstream origin backport-63358-to-5.4.5099
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.4.5099
Once the pull request has been created, please ensure the following:
|
This PR refactors the
CompletionsClientinterface, and all the corresponding call sites. There is no functional change, beyond bundling several function parameters into a new type.See
internal/completions/types/types.go. But the gist is this putting 3x parameters into a singleCompletionRequesttype.Complete( context.Context, log.Logger - CompletionsFeature, - CompletionsVersion, - CompletionRequestParameters + CompletionRequest ) (*CompletionResponse, error)Why?
As part of reworking the codepath between receiving a completion request, dispatching it to the right
CompletionsClientimplementation, and serving the request, I need some "hooks" to inject new information.In a future PR I plan on adding a
*ServerSideModelConfigas another field to theCompletionRequest, so that when theCompletionClient's implementation is trying to serve that request it has any additional data it needs. (For example, the AWS Bedrock provisioned capacity ARN, etc.)Test plan
Updated existing tests, relying on CI/CD for any other issues.
Changelog
NA, just some under the hood refactoring that shouldn't impact any functionality.