Skip to content

Conversation

mwillfox
Copy link
Contributor

Non-breaking changes which allow for a configurable "Transport" (language which was already used in the project, really a *http.Client).

This allows for setting a custom RoundTripper or using github.com/hashicorp/go-retryablehttp:

transport := retryablehttp.NewClient().StandardClient()
c := gogpt.NewClientWithTransport(openAIAPIKey, transport)

With the recent stability issues and load of the OpenAI API this is necessary to use the module in a production environment.

@sashabaranov
Copy link
Owner

Hey, thank you for that PR! It made me think about how we want to approach client initialization — there was a great article about different ways of doing that, but I could not find it. The closest is this one: https://asankov.dev/blog/2022/01/29/different-ways-to-initialize-go-structs/

I don't think the usage of .WithA().WithB().WithC() chaining is a way forward for us here. I would rather prefer an API akin to:

client := gogpt.NewClient()

// or

config := gogpt.DefaultConfig()
config.Transport = retryablehttp.NewClient().StandardClient()
client := gogpt.NewClientWithConfig(config)

@mwillfox would you have the bandwidth to do such a refactoring? It's totally cool if not — I can do it in a follow-up PR.

@mwillfox
Copy link
Contributor Author

I should have time to refactor in the next few days and I'll update the PR. Thanks for the feedback!

@sashabaranov sashabaranov changed the base branch from master to better-config February 19, 2023 10:59
@sashabaranov
Copy link
Owner

Will follow up this work in a better-config branch

@sashabaranov sashabaranov merged commit 789b9ad into sashabaranov:better-config Feb 19, 2023
@sashabaranov sashabaranov mentioned this pull request Feb 19, 2023
sashabaranov added a commit that referenced this pull request Feb 20, 2023
* Configurable Transport (#75)

* new functions to allow HTTPClient configuration

* updated go.mod for testing from remote

* updated go.mod for remote testing

* revert go.mod replace directives

* Fixed NewOrgClientWithTransport comment

* Make client fully configurable

* make empty messages limit configurable #70 #71

* make auth token private in config

* add docs

* lint

---------

Co-authored-by: Michael Fox <[email protected]>
Copy link

@Eriddle96 Eriddle96 left a comment

Choose a reason for hiding this comment

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

This is a great idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants