separate read + connect + request timeouts

[thehesiod]

This is an attempt at fixing the timeout issues raised in #1524 + #1180

[codecov-io]

Codecov Report

Merging #1523 into master will increase coverage by <.01%.

@@            Coverage Diff             @@
##           master    #1523      +/-   ##
==========================================
+ Coverage   98.59%   98.59%   +<.01%     
==========================================
  Files          30       30              
  Lines        7055     7071      +16     
  Branches     1176     1179       +3     
==========================================
+ Hits         6956     6972      +16     
  Misses         58       58              
  Partials       41       41

Impacted Files	Coverage Δ
aiohttp/client.py	100% <100%> (ø)	✅
aiohttp/connector.py	97.87% <100%> (+0.01%)	✅

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1811685...94af4b5. Read the comment docs.

[thehesiod]

I updated the implementation to allow for unlimited timeouts by passingrequest(timeout=None)

[adamrothman]

@jettify @asvetlov what do you guys think about this? Would make it a lot easier to use aiohttp in more places.

[thehesiod]

adding @fafhrd91

[fafhrd91]

looks good. i am not sure about proper api though. lets add read_timeout but lets wait with documentation.

i did some work on TimeService but for server, we may introduce time_service for client as well, so it'd be possible to use third-party time service.

[thehesiod]

I think it was a mistake making the default _request timeout 5min, it should have been None and the details handled internally, it makes backwards compatible fixes difficult as we can't differential between the user setting it to 5minutes, or them wanting the default behavior.

[thehesiod]

cool, let me know what you want me do.

[fafhrd91]

request timeout 5min where is it? if we have 5min timeout, it should painless to replace it with None i doubt anyone notice.

[fafhrd91]

i'll merge this change. if you'd like you can look into time_service integration

[fafhrd91]

thanks

[thehesiod]

https://github.com/KeepSafe/aiohttp/pull/1523/files#diff-7dd84b5ef8d5eea2de1dfc5329411dfcL127 let me know if you want me change it to None and update the logic

[fafhrd91]

ok, lets replace it with None

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.