search

UMIACS / rgwadmin

Ceph Object Storage Admin API python library bindings.
https://rgwadmin.readthedocs.io/
GNU Lesser General Public License v2.1
85 stars 47 forks source link

add timeout for requests #21

Closed jacek-jablonski closed 6 years ago

jacek-jablonski commented 6 years ago

Currently requests have no timeout (by default in requests lib). Hanged RGWs may keep REST API tasks running unnecessarily long. Adding timeout prevents such situations.

robbat2 commented 6 years ago

LGTM

liammonahan commented 6 years ago

Thanks!

liammonahan commented 6 years ago

@jacekjab Do you think maybe we should set the default to some conservative number instead of None by default? My guess is that there aren't any admin operations that take very long. Maybe 6 seconds?

robbat2 commented 6 years ago

On a large cluster, admin operations to list things CAN take a long time. Listing metadata (see my PR) can be very slow. I consider this partially a problematic design choice: some operations simply cannot be made faster, but an HTTP API doesn't provide a good way to do callbacks, so you're stuck with polling for completion instead.

jacek-jablonski commented 6 years ago

@liammonahan as @robbat2 mentioned some metadata requests can take very long time. So as for me conservative number is rather 120 seconds.

liammonahan commented 6 years ago

That's for the context @jacekjab and @robbat2 I think it's hard to pick what the perfect number would be in this case, so I'm thinking we keep it as is in the PR and let the caller have the ability to specify the timeout without providing one explicitly.