Hi Kerrat, Please see my comments in the doc - I think I commented while this email went out. I would prefer to just retry on UNAUTHENTICATED.
Also, with gRPC and especially with streaming calls, you may not get an error status right away, as you've found - so there's additional work needed to handle that. In our use we've generally assumed that if the server will respond with UNAUTHENTICATED, it'll do so before returning any data (so there's no need to look for it once the stream starts). Best, David On Wed, Nov 25, 2020, at 14:39, Keerat Singh wrote: > Hey Wes, > > Renaming it to be more generic like AUTH_EXPIRED sounds good to me, updated > the design doc. > RE: The implementation, while working on the Java POC for this: > > - Adding it to the FlightStatusCode > > <https://github.com/apache/arrow/blob/47b2dd57cdb7098b3c533fb25055df66e1b9c3d0/java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightStatusCode.java> > and CallStatus > > <https://github.com/apache/arrow/blob/47b2dd57cdb7098b3c533fb25055df66e1b9c3d0/java/flight/flight-core/src/main/java/org/apache/arrow/flight/CallStatus.java> > was quite straightforward. > - However, the challenge that I saw was that when an API call like > listFlights returns an Iterable, the underlying FlightServiceGrpc method is > not called until we are traversing the iterator in the iterable. > - The exception returned while traversing the iterator is a > StatusRuntimeException [Link] > > <https://github.com/apache/arrow/blob/47b2dd57cdb7098b3c533fb25055df66e1b9c3d0/java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/StatusUtils.java#L216>, > which means we don't have the AUTH_EXPIRED Status code unless we add the > code to the io.grpc.Status > > <https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/Status.java> > class. > @David Do you have any thoughts on this? > > > - The other alternate I was thinking of was to retry whenever we use the > bearer credential header and it returns an UNAUTHENTICATED status code. > @David Do you have any thoughts on this? > > Regards, > Keerat > > > On Wed, Nov 25, 2020 at 11:19 AM Wes McKinney <wesmck...@gmail.com> wrote: > > > In principle adding a new error code for this seems reasonable to me. > > What do you think about calling it something more generic like > > "AUTH_EXPIRED"? I haven't looked at the details of the implementation > > -- David Li or others may be able to provide better comments? > > > > On Fri, Nov 20, 2020 at 6:23 PM Keerat Singh <keer...@bitquilltech.com> > > wrote: > > > > > > I would like to propose the following enhancement to retry the failed > > > Flight API call and refresh the access token on encountering > > > FlightStatusCode.TOKEN_EXPIRED. > > > For example when using an implementation of CallHeaderAuthenticator > > > < > > https://github.com/apache/arrow/blob/master/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/CallHeaderAuthenticator.java > > > > > > such as BearerTokenAuthenticator > > > < > > https://github.com/apache/arrow/blob/master/java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthenticator.java > > > > > > and the generated access token expires and FlightClient needs to detect > > > that and retry the failed API call, along with the retry it needs to > > > refresh the expired access token rather than having each client > > application > > > write their own custom code with retry logic around this token expired > > > workflow. > > > > > > More Details: > > > > > > - Arrow JIRA: https://issues.apache.org/jira/browse/ARROW-10671 > > > - Design Proposal: > > > > > https://docs.google.com/document/d/187DlGpIpOUPGhWvXVQEq0mXw_hdWjzzOuZp0p5qzBp0/edit?usp=sharing > > > > > > Regards, > > > Keerat > > >
