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

support configure HostClient #1214

Merged
merged 1 commit into from Feb 15, 2022

Conversation

farss
Copy link
Contributor

@farss farss commented Feb 10, 2022

ConfigureClient configures the fasthttp.HostClient.
So I can use https://github.com/dgrr/http2 in fasthttp.Client

@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Feb 10, 2022

I'm curious, what do you need to configure that you can't configure on the fasthttp.Client itself?

@farss
Copy link
Contributor Author

farss commented Feb 11, 2022

I'm curious, what do you need to configure that you can't configure on the fasthttp.Client itself?

like: HostClient.Transport

@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Feb 12, 2022

In that case I would prefer it if you added Client.Transport that gets copied to the HostClient then, just like all the other options.

@farss
Copy link
Contributor Author

farss commented Feb 14, 2022

Client

In that case I would prefer it if you added Client.Transport that gets copied to the HostClient then, just like all the other options.

Client don't know Addr of HostClient, So need callback function to config HostClient

@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Feb 14, 2022

I'm not sure I understand. HostClient.Transport is a func(*Request, *Response) error. Why can't that be configured on Client and passed to HostClient? In the Transport function you get the Request so you can make decisions based on that?

@farss
Copy link
Contributor Author

farss commented Feb 15, 2022

HostClient.Transport works on all requests. In my situation only want to config HostClient it self.

@erikdubbelboer erikdubbelboer merged commit c94581c into valyala:master Feb 15, 2022
10 of 11 checks passed
@erikdubbelboer
Copy link
Collaborator

erikdubbelboer commented Feb 15, 2022

Ok you convinced me 😄

@farss
Copy link
Contributor Author

farss commented Feb 15, 2022

thank you!

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.

None yet

2 participants