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

Timeout as TimeSpan, Support custom request timeout #2078

Merged
merged 1 commit into from
May 22, 2024

Conversation

RomanSoloweow
Copy link
Contributor

@RomanSoloweow RomanSoloweow commented May 6, 2023

  1. I think the current approach to use int for timeout is outdated and it generates a lot of errors when you need to remember that these are milliseconds, and you need translate it. Even HttpClient has timeout as TimeSpan.

  2. The presence of "MaxTimeout" also confuses many users. If you want to set a custom timeout for the request, you need to remember which timeout is set for the rest client. Often, when you set a custom timeout for request, it's higher timeout than the default. These are requests for downloading and uploading files. It's rare case when the timeout is specially reduced for a particular request.

@RomanSoloweow
Copy link
Contributor Author

@alexeyzimarev

tests failed again. Pull request does not affect this.
image

@RomanSoloweow
Copy link
Contributor Author

@alexeyzimarev ping

1 similar comment
@RomanSoloweow
Copy link
Contributor Author

@alexeyzimarev ping

@alexeyzimarev
Copy link
Member

I am not merging it because it will break things. Still thinking how to make it available with some transition.

@RomanSoloweow
Copy link
Contributor Author

@alexeyzimarev any news?

@alexeyzimarev alexeyzimarev merged commit 9117b7a into restsharp:dev May 22, 2024
2 of 3 checks passed
@SergeiPavlov
Copy link

SergeiPavlov commented May 24, 2024

This change https://github.com/restsharp/RestSharp/pull/2078/files#diff-e00edb0ffd0c39411486e45e69f01fc071d99a607cd06c9e27acebdd1241939cR221

broke typical usage pattern new RestClient(sharedHttpClient,... because the HttpClient.Timeout property cannot be changed after HttpClient has started

@alexeyzimarev
Copy link
Member

I noticed that one and flagged it, but for some reason I didn't think that ConfigureHttpClient is called when an external instance is used.

I will release a fix (the code is already fixed).

I think ConfigureHttpClient needs to be removed as all what's left there is setting Expect100Continue and it's a header.

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

3 participants