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

use reqwest::blocking::Client with no timeout where the default 30s might not be enough #127

Conversation

CrazyByDefault
Copy link

according to request::blocking::get() docs, the request will fail if the download takes longer than 30s.

this PR implements a basic reqwest::blocking::Client with timeout=None for this request in particular

fixes #126

@CrazyByDefault CrazyByDefault changed the title use reqwest::blocking::Client with a 90s timeout where the default 30s might not be enough use reqwest::blocking::Client with no timeout where the default 30s might not be enough Jun 28, 2024
@JayAndromeda
Copy link
Contributor

hello, i do not agree with setting the timeout to 'None' for the repo. it is not a good practice to fully disable a request timeout in general. if you are running into timeouts for yourself frequently then id look into shelving this commit locally so that it works for you but a 30s timeout is plenty for 99% of scenarios. on top of that im sure bungie has a connection timeout on their end as well but we shouldnt rely on that.

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.

General: build.rs fails to download DestinyInventoryItemDefinition due to TimedOut
2 participants