Custom http headers #93
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e. Refactor URLRequestBuildable and its implementations to separate configuration concerns from URL building concerns.
… that the code better adheres to the Single Responsibility Principle.
…nly be used by the OpenAI class.
…o enable future extension and/or testing.
|
Kudos, SonarCloud Quality Gate passed!
|
|
Hi! Is there anything more I need to do in order to get this merged? |
|
Hey, @benvolioT! |
| // on the next pass in case that new data has completed the string to create valid (parsable) JSON, and so on. | ||
| // Note that while OpenAI's API doesn't appear to return partial string fragments, proxy service such as Helicone | ||
| // do seem to do so, making this logic necessary. | ||
| accumulatedData.append(stringContent) |
There was a problem hiding this comment.
Could you extract this change to a separate PR, please?
Looks like this is a separate fix, that doesn't apply to custom headers.
| } | ||
|
|
||
| extension OpenAI { | ||
| internal func generateHeaders() -> [String: String] { |
There was a problem hiding this comment.
I would change the naming to defaultHeaders to add the knowledge that it is going to be appended further.
What do you think?
Krivoblotsky
requested changes
Aug 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
This pull request adds the ability to specify custom HTTP headers when integrating with OpenAI via proxy APIs such as Helicone. It also refactors OpenAi's performRequest() by encapsulating the logic for handling the session data task in its own class, URLSessionDataTaskManager. This should simplify the OpenAI class and make the code easier to manage. This pull request simplifies JSONRequest and MultipartFormDataRequest by abstracting out common code to remove repetition that previously existed in these structs. Finally, it simplifies the protocol URLRequestBuildable, moving configuration-driven parameter setting to the constructors of these struct instances, rather than in the call to that protocol's build() function.
Why
The motivation for this change was to allow users to integrate with OpenAI via the Helicone proxy. To do so, they not only need to point the OpenAI client at the Helicone host, but they also have to set an additional HTTP header on the requests. This pull request makes that possible.
Affected Areas
The affected areas of the library include: