Sounds good - it would be good to have that context available. Thanks, David
On Thu, Feb 17, 2022, at 16:19, José Almeida wrote: > Awesome, > > Our intention is not to make FlightSQLClient always wrap the > RetryingFlightClient, but give the possibility to both have this > functionality. So making the changes in the FlightClient makes more sense. > > Regarding the document, I can create a new one and attach it with the PR, > is that ok? > > - Jose Almeida > > > > On Thu, Feb 17, 2022 at 5:40 PM David Li <lidav...@apache.org> wrote: > >> 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 >> > >> >> >>