That sounds reasonable. I would expect this gets implemented as a wrapper on top of FlightClient, since that logic could be useful in contexts beyond Flight SQL. Though then we need to do work to extract an interface out of FlightClient in C++/Java (unless the intent is that FlightSqlClient will always wrap a RetryingFlightClient?).
I note the doc in that PR has been deleted - could the proposal be written up in more detail? (Though I suppose this is not really a format or protocol change.) But it sounds like we want to have a standard auth scheme for Flight SQL, and in that case we should write it out. (Also nit: maybe "RetryingFlightClient" makes more sense?) -David On Thu, Feb 17, 2022, at 13:51, José Almeida wrote: > So after your reply we discussed it and came up with a suggestion and a > question. > > First, we would like to know where is the > best place to implement it, in FlightClient itself or in the > FlightSQLClient. > > Our idea for implementing it is to create a new class > "RetryableFlightClient" which will > wrap all the calls from FlightClient (except the authentication ones). If > it fails during the > first attempt because of an authenticated status, a clientMiddleware (that > will be created) > will be used to authenticate with the basic credentials and generate a new > bearer token. > The same can work for FlightSQLClient if it should only be implemented on > it. > > Link to the original proposal -> https://github.com/apache/arrow/pull/8780 > > - Jose Almeida > > On Tue, Feb 8, 2022 at 3:37 PM David Li <lidav...@apache.org> wrote: > >> For gRPC, in theory, you can get UNAUTHENTICATED at any time, including >> after the client has gotten some results. >> >> If you need to retry calls, and you want to do it transparently, you'd >> need a gRPC interceptor, yes. (The Flight middleware is not powerful enough >> to do that.) But that should be separable from authentication itself? >> >> As a side note, we have far too many auth methods, especially as some are >> misleadingly named (e.g. the "basic" auth has little to no relation with >> HTTP Basic Auth). I suppose a lot of it is just historical stuff that >> should probably be cleaned up, or at least properly documented. >> >> -David >> >> On Tue, Feb 8, 2022, at 13:15, José Almeida wrote: >> > Hi guys, We are assuming the Bearer Token Refresh task, which was started >> > but now it's been paused for a while (link to original POC) >> > <[link](https://github.com/apache/arrow/pull/8780)>, and we have some >> > concerns about it, such as: >> > >> > 1 During a Flight Call we can get unauthenticated while consuming a >> stream >> > or just when an operation is started? We were wondering if the creation >> of >> > a wrapper around the StreamObserver is needed. >> > 2 Would it be better for an Interceptor to make this Authentication? We >> > were basing ourselves on this comment >> > <https://github.com/grpc/grpc-java/issues/5856#issuecomment-511077021> >>
