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

Add a way to configure a timeout for an API call #98

Open
arthuraraujo-msft opened this issue Feb 29, 2024 · 2 comments
Open

Add a way to configure a timeout for an API call #98

arthuraraujo-msft opened this issue Feb 29, 2024 · 2 comments
Labels
nice to have This is not required, but would be nice to have
Milestone

Comments

@arthuraraujo-msft
Copy link
Contributor

          I'm confused after reading the implementation in CurlConnection.cpp. `maxRequestDuration` does appear to somewhat control the overall duration of the `GetLatestDowloadInfo` calls but not exactly. Why not change maxRequestDuration to mean the overall operation timeout and set a timeout for each "curl perform" equal to `min(<per-request-duration>, maxRequestDuration)? After a request fails, check your stopwatch against maxRequestDuration to decide whether to retry or not.

Originally posted by @shishirb-MSFT in #90 (comment)

@arthuraraujo-msft arthuraraujo-msft added this to the Future milestone Feb 29, 2024
@arthuraraujo-msft arthuraraujo-msft added the nice to have This is not required, but would be nice to have label Feb 29, 2024
@shishirb-MSFT
Copy link

shishirb-MSFT commented Feb 29, 2024

Something like this:

struct RequestParams
{
    ...

    // Max time that this request is allowed to take. SFS client is free to perform
    // retries, control per-HTTP request timeout, etc., on its own while adhering 
    // to this overall timeout.
    std::chrono::milliseconds timeout = 30s;
};

@shishirb-MSFT
Copy link

shishirb-MSFT commented Feb 29, 2024

The bool retryOnError that is being added in #90 can be retained even after adding this timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nice to have This is not required, but would be nice to have
Projects
None yet
Development

No branches or pull requests

2 participants