Let's name this something more specifc (retryOnUnauthorized perhaps)

I believe you don't have to worry about this since you will either get the first result or UNAUTHENTICATED. (Or else, you'd have a very odd server that explicitly sends an UNAUTHENTICATED after giving you results.)

This should update this.itr so that subsequent calls use the new call.

nit: can we keep getters after fields/constructor?

A general comment in the interests of code reuse; you may be better off implementing a generic StreamObserver which can wrap any gRPC call. You'll have to change FlightClient to always use the async gRPC stub instead of the blocking stub, but then you can handle retries for any gRPC call without having to write special-purpose wrappers for each call.

@lidavidm Thank you for your feedback

I have pushed a PS with the suggested changes.

I do currently have a blocking call [link] on the API calls that return an Iterable, like listFlights and listActions, as we need to have the iterable populated before we return it to the FlightClient, if not then we are back to the same approach using a wrapped iterator.

While the changes latest PS doClientCall only caters to API calls that return an iterable/iterator, the next step is to make them generic for calls like getInfo, getSchema
Before I go ahead and do that, I wanted to confirm if this is what you had in mind when you made the suggestions above, regarding a generic StreamObserver.

Other options that I had in mind were to pass the functional interfaces to pass the ApiCallObserver and have it retry the API call onError, in that case, we would not need to block doClientCall until the ApiCallObserver completes as the API call would be retried asynchronously when the Observer encounters an error.

Sorry - let me clarify. What I mean is that gRPC offers 3 types of clients in Java: a blocking client, async client, and a "generic" client. This last client offers a unified interface for all types of calls: it accepts a StreamObserver which gets called with data from the server and it is given a StreamObserver to send data to the server. So if you were to rewrite all gRPC calls in terms of this last type of client, then you could implement a single StreamObserver wrapper that handles retries for all calls.

Not a requirement - just offering a suggestion to avoid having to implement lots of single-purpose retries. It would take a decent amount of refactoring.

So I see you have switched to using the generic client; my comment is then more about the level of abstraction appropriate for implementing retry. In particular, keeping the logic inside a wrapper StreamObserver would, IMO, make it easier to preserve things like the streaming nature of calls without having to buffer results.

This should only retry on the appropriate status right?

Also, doesn't this need to somehow reset the middleware so that it sends the basic auth credentials and not the bearer token?

Can we use a condition variable instead of busy-waiting?

This means we now build up the entire result set in memory instead of being able to stream it.

nit: can we make this more descriptive (this is a set of utilities for making RPC calls with optional retries)?

nit: more specifically, this converts an async callback-based call into a synchronous iterator

Can we make it clear here that this class generates bearer tokens with a sequentially increasing ID, and it considers token #1 to be 'expired'? It was a little unclear how the retry was getting tested at first.