feat: Retry handling for Clients

[santhoshramaraj]

• This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

Exposes retry handling from Uplink and example to implement retry handling with clients.

Why should this Pull Request be merged?

To develop robust client applications, users must handle common scenarios such as Too Many Requests and https disconnections when using the SystemLink clients.

What testing has been done?

TBD

[rbell517]

While this approach will work, its pretty kludgy to invoke the retry decorator in this way. I would prefer a new function on BaseClient like with_retry so your example would instead look like:

client = FileClient().with_retry(when= ...)
# OR because `retry` decorator modifies the class
client = FileClient()
client.with_retry(when=...)

T = TypeVar('T', bound='BaseClass')

class BaseClient(Consumer):
    ...
    def with_retry(
        self: T, 
        when=None,
        ...
    ) -> T:
        return retry(
            when,
            ...
        )(self)

And ideally we'd type these retry parameters properly, even though uplink does not.

One thorn in the side for this approach is subsequent calls to with_retry would likely behave strangely, from my understanding of how uplink is handling these annotations. I believe you'd end up with multiple instances of the retry handler applied when the requests are ultimately built. This is a problem with your original approach, too, but its less likely to be a a problem since you're doing this at construction.

This duplicate handlers problem is annoying because it would make adding a default retry behavior that you could then override with with_retry not work properly, and I think having a default is the direction we should go in based on feedback from @ChrisStrykesAgain. If we are sure we will not use the @retry decorator and always use this new with_retry function we can simply keep hold on that retry object and update it like so

T = TypeVar('T', bound='BaseClass')

class BaseClient(Consumer):
    ...
    def with_retry(
        self: T, 
        when=None,
        ...
    ) -> T:
        retry = retry(
            when,
            ...
        )
        if self._retry is None:
            self._retry = retry
            # Bind to this client's methods
            retry(self)
        else:
            # replace the already bound retry annotations config
            self._retry._when = retry._when
            self._retry._backoff = retry._backoff
            self._retry._stop = retry._stop
        
        return self

Or we can punt on subsequent updates until we also add default retries and for now just throw if you try to set the retry config more than once to prevent unexpected behavior. Making a new client with a different retry is pretty trivial after all.

[rbell517]

As I said in my other comment, we'd like to have a default pretty good retry config that is useful in most cases that is simple to add and can be updated as we find cases to include. Could you take a crack at making one?

Retrying 502 and 503 would be good, and 429 as you have. I'm not sure what Exception cases you're trying to retry, did you have something in mind? I would start without that. We definitely want to stop after a number of retries, but I think we can leave the delay off and just rely on the count of retries plus backoff to get to the time we want.