nens / threedi-api-client

Documentation
https://threedi-api-client.readthedocs.io
Other
0 stars 1 forks source link

Add retry functionality #64

Closed caspervdw closed 3 years ago

caspervdw commented 3 years ago

This adds functionality for retrying requests to the threedi-api-client. Example usage:

from threedi_api_client import ThreediApi

api = ThreediApi(
    env_file=".env",
    version="v3-beta",
    rest_config={"retries": 3},  # or a urllib3.util.Retry instance for more functionality
)

For asynchronous operation, I had to hack a bit into the generated client. I made use of aiohttp_retry. To minimize the access to private properties (which would give instability across versions) I integrated the code into threedi-api-client and adapted the call signature of the RetryClient so that it wraps the ClientSession which is initialized by the generated code. This now works:

from threedi_api_client import ThreediApi
from threedi_api_client.aio.aiohttp_retry import ExponentialRetry

api = ThreediApi(
    env_file=".env",
    version="v3-beta",
    rest_config={"retries": ExponentialRetry(attempts=3)}, 
)
jpprins1 commented 3 years ago

What's your opinion about using one of the retry policies as default?

caspervdw commented 3 years ago

Looks good, just have some questions (for my understanding):

* Why do you need to create a list of urls from a single url?

This is part of the code from aiohttp_retry, you probably had a closer look then I have had.

* Did you override the whole rest stuff to provide a more granular interface for setting the retry policy?

For the async client, the configuration.retries was just ignored. I had to override the pool_manager so that something happened.

* Subclassing the `RestClientObject` won't work? ( https://github.com/nens/threedi-api-client/blob/b49b7481e7b83d2d0fde64153559ce3e31085ede/threedi_api_client/openapi/rest.py#L51
  )

I don't know how to inject the subclass then, this is also in generated code: https://github.com/nens/threedi-api-client/blob/b49b7481e7b83d2d0fde64153559ce3e31085ede/threedi_api_client/openapi/api_client.py#L76

caspervdw commented 3 years ago

What's your opinion about using one of the retry policies as default?

That's a good idea, I defaulted to 3, which is what urllib3 does too.

Also I changed the API a bit, it is now a retries parameter instead of rest_config["retries"]. I see no other usage of rest_configcurrently and we can easily add more parameters if necessary.