Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix(cody-gateway): getAPIURL before transformBody#63406

Merged
abeatrix merged 2 commits into
mainfrom
bee/fix-upstream-stream
Jun 20, 2024
Merged

fix(cody-gateway): getAPIURL before transformBody#63406
abeatrix merged 2 commits into
mainfrom
bee/fix-upstream-stream

Conversation

@abeatrix

Copy link
Copy Markdown
Contributor

Fix an issue where the requestBody is used after transformBody has been executed.

Test plan

  1. Start Cody Gateway locally
  2. Start SG local dev instance
  3. Connect SG local dev instance to your local Cody Gateway instance
  4. Set Gemini Flash as your chatModel
  5. Connect Cody to your local dev instance
  6. Ask Cody a question and verify you are getting a response

image

Changelog

@cla-bot cla-bot Bot added the cla-signed label Jun 20, 2024
@abeatrix abeatrix force-pushed the bee/fix-upstream-stream branch from a388898 to 4bff03d Compare June 20, 2024 22:55
}

// Get the URL to call for the upstream provider before we transform the request.
upstreamURL := methods.getAPIURL(feature, body)

@abeatrix abeatrix Jun 20, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was issue #1, where before this change, we would use the transformed body to getAPIURL (currently only Google uses body to build the APIURL), and in Google case, we have already stripped the Stream value, so body.Stream was always returning false.


// Store the shouldStream value in case it changes during the transformation.
// Example: We remove it for Google requests.
shouldStream := body.ShouldStream()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is issue #2, where before this change, we would call the body.ShouldStream() that returns body.Stream value, which could have changed already during the transformBody step. Moving this before transformBody resolve the issue.

@abeatrix abeatrix requested review from a team, arafatkatze and emidoots June 20, 2024 23:03

@chrsmith chrsmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for calling out the nuance of the change. This seems reasonable.

@abeatrix abeatrix merged commit b3fe6dc into main Jun 20, 2024
@abeatrix abeatrix deleted the bee/fix-upstream-stream branch June 20, 2024 23:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants