search

NVIDIA / aistore

AIStore: scalable storage for AI applications
https://aistore.nvidia.com
MIT License
1.21k stars 160 forks source link

python: add timeout option to client #173

Closed timoha closed 5 months ago

timoha commented 5 months ago

By default internally used requests library doesn't set any timeouts which means that if server doesn't reply to request, call will just hang.

We've run into this issue on our cluster and need an ability to have requests fail after some time of inactivity.

In general, it's a good practice to set default timeouts for any clients that do requests over network. This change introduces an option to set a timeouts as a float, or a (connect timeout, read timeout) tuple that will be applied to all requests done by the client.

The default remains as None to keep backwards compatibility.

Their documentation requests also recommends setting timeouts to most external request: https://requests.readthedocs.io/en/latest/user/advanced/#timeouts

gaikwadabhishek commented 5 months ago

Hi @timoha! Thank you for your contribution. PR overall looks good. Can you please sign the commit before we merge it?

timoha commented 5 months ago

Hi @timoha! Thank you for your contribution. PR overall looks good. Can you please sign the commit before we merge it?

thank you, signed-off! Updated tests as well.

gaikwadabhishek commented 5 months ago

run make fmt-fix to fix the python formatting

gaikwadabhishek commented 5 months ago

Hey @timoha! I have merged your commit through the internal pipeline. I have also made a release of PyPI that has you changes - https://pypi.org/project/aistore/1.4.22/

Thank you for your contribution!