Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP client factory for per-request clients #147

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ash2k
Copy link
Contributor

@ash2k ash2k commented Oct 19, 2021

Please see #139 (comment) for the background.

This approach allows the library user to decide how they want to handle any client/transport related concerns. It decouples the library from those concerns as it doesn't have to have an opinion on how keep-alives need to work. Different situations need different behavior so giving users full control is the best option.

@ash2k
Copy link
Contributor Author

ash2k commented Oct 19, 2021

@ryanuber PTAL

@ash2k ash2k mentioned this pull request Oct 19, 2021
type CleanPooledClientFactory struct {
}

func (f *CleanPooledClientFactory) New() HTTPClient {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something to consider is adding an error to the returned values. Some factories may have a problem while constructing a client and they will not have a good way to propagate the error up. WDYT?

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@ash2k
Copy link
Contributor Author

ash2k commented Sep 1, 2022

Rebased. This is ready for review/merge.

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

Successfully merging this pull request may close these issues.

3 participants