You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by José Almeida <jo...@simbioseventures.com> on 2022/02/08 18:15:48 UTC
Bearer Token Refresh Task
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>
Re: Bearer Token Refresh Task
Posted by David Li <li...@apache.org>.
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 <li...@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 <li...@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
>> >
>> >>
>>
Re: Bearer Token Refresh Task
Posted by José Almeida <jo...@simbioseventures.com>.
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 <li...@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 <li...@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
> >
> >>
>
Re: Bearer Token Refresh Task
Posted by Holden Karau <ho...@pigscanfly.ca>.
While we’re talking about this, for engines like Spark/Flink/Dask/Ray where
we might have many executors does it make sense to have centralized refresh
instead of each executor doing a refresh on token expiration?
On Thu, Feb 17, 2022 at 12:40 PM David Li <li...@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 <li...@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
> >
> >>
>
--
Twitter: https://twitter.com/holdenkarau
Books (Learning Spark, High Performance Spark, etc.):
https://amzn.to/2MaRAG9 <https://amzn.to/2MaRAG9>
YouTube Live Streams: https://www.youtube.com/user/holdenkarau
Re: Bearer Token Refresh Task
Posted by David Li <li...@apache.org>.
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 <li...@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>
>>
Re: Bearer Token Refresh Task
Posted by José Almeida <jo...@simbioseventures.com>.
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 <li...@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>
>
Re: Bearer Token Refresh Task
Posted by David Li <li...@apache.org>.
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>