search

beetbox / pyacoustid

Python bindings for Chromaprint acoustic fingerprinting and the Acoustid Web service
MIT License
325 stars 66 forks source link

HTTP Request timeout #53

Closed Javinator9889 closed 4 years ago

Javinator9889 commented 4 years ago

For slow connections, maybe a request can take too much time to complete. In addition, when AcoustID web service is not available (due to maintenance, connection error, etc.), the request takes "infinite" to complete. By passing the timeout param to the Session object, the request can finish when it is not completed or go infinite if it is its default value (None). Then, when failing, raises a standard TimeoutError so the developer can handle it without using the requests library

sampsyo commented 4 years ago

Looks good overall! Just one thing I think we should do differently: I think we should raise a WebServiceError (or a subtype thereof) instead of Python's built-in TimeoutError. This way, existing code can get by with just handling WebServiceErrors and catch both cases where the server has a bug and returns a 500 error, for example, and when the server is down entirely. Furthermore, it looks like Python's TimeoutError is meant for OS timeouts; I don't see any indication that it's supposed to be used for general-purpose timeout indications.

Javinator9889 commented 4 years ago

@sampsyo changed :) Now the only exception that is raised is WebServiceError

sampsyo commented 4 years ago

Awesome; thank you for doing this!!