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

change req.timeout to public and add min deadline in pipeline #1344

Closed
wants to merge 2 commits into from

Conversation

fare83
Copy link
Contributor

@fare83 fare83 commented Jul 12, 2022

change req.timeout to public for Do method support timeout;
add timeout in clientGetURLDeadline;
add min deadline in pipeline

@erikdubbelboer
Copy link
Collaborator

Can you explain why you need Request.Timeout to be public?

@fare83
Copy link
Contributor Author

fare83 commented Jul 14, 2022

when i use hostclient.Do ,i can't set timeout

@erikdubbelboer
Copy link
Collaborator

But that internal timeout is only used as a timeout to acquire a connection if you have MaxConns set. What I would actually much rather do is make that timeout equal to WriteTimeout and completely get rid of req.timeout.

@fare83
Copy link
Contributor Author

fare83 commented Jul 17, 2022

I want to set timeout for current request, but writetimeout is for all host and request.

@erikdubbelboer
Copy link
Collaborator

What do you think of this instead: #1346
HostClient already has DoTimeout and DoDeadline. What this pull request does it just make them use req.timeout instead of the separate goroutine that I have always hated.

@fare83
Copy link
Contributor Author

fare83 commented Jul 28, 2022

#1346 LGTM

@fare83 fare83 closed this Jul 28, 2022
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