Open mykola-mokhnach opened 1 year ago
@mykola-mokhnach good point, looks like we've ended up with some weird multiple layers of timeout overrides at this point. We're about to release a major version bump -- sounds like today is a good day to introduce the breaking change to fix this!
Thanks for calling this out.
Turns out the plumbing for this is actually quite difficult -- I got started in an implementation here, but the rest-vs-aio code divide and the variations in how requests and aiohttp handle differing value is quite the nightmare here. I won't be targeting this for the upcoming major, but will keep thinking about it (and of course, happy to review any PRs or take any suggestions!).
It's likely we'll need to do some refactoring before this is easy enough to implement; one idea we've tossed around is to migrate to httpx
(since it supports aio and sync modes), which would let us remove a whole lot of the build-time variances between libraries. Doing that first might save a whole lot of effort.
All public methods of Storage class have a default timeout value of type
int
. This value effectively overrides any timeout values provided to Session object on Storage creation.I would expect the timeout value to be optional, so it does not always override default session timeouts implicitly (10 seconds is too low for generic downloads and uploads).