Hi, > It is generally recommended by the gRPC authors to have > separate request and response messages for each RPC
Oh, I didn't know about it. Should we create CancelFlightInfoRequest/CloseFlightInfoRequest messages instead of using FlightInfo directly for CancelFlightInfo/CloseFlightInfo? Thanks, -- kou In <0add20bd-34af-425f-8bdd-bcc609427...@app.fastmail.com> "Re: [DISCUSS][Format][Flight] Result set expiration support" on Wed, 21 Jun 2023 21:49:58 -0400, "David Li" <lidav...@apache.org> wrote: >> So this may not be needed for now. How about accepting a >> specific request message instead of FlightEndpoint directly >> as "PersistFlightEndpoint" input? > > I'm in favor of this on principle. It is generally recommended by the gRPC > authors to have separate request and response messages for each RPC, since it > gives you flexibility in cases like this. It does lead to some possibly > unnecessary indirection, but can save us pain down the line. > >>> * "RefreshFlightEndpoint" suggests the server will recompute (refresh) >>> the results; instead I would suggest "PersistFlightEndpoint" > > I think Refresh was fine, but if there's confusion, I like Kou's suggestion > of Renew the best. > > On Wed, Jun 21, 2023, at 20:58, Sutou Kouhei wrote: >> Hi, >> >>> * "RefreshFlightEndpoint" suggests the server will recompute (refresh) >>> the results; instead I would suggest "PersistFlightEndpoint" >> >> Thanks for the suggestion. But I feel that "persist" doesn't >> have expiration time. (NOTE: I'm not a native English >> speaker.) >> >> How about the followings? >> >> * "ExtendFlightEndpoint" >> (We may want to add "expiration" to the name too but it >> may be long...) >> * "RenewFlightEndpoint" >> (It may suggest "recompute" too...) >> >>> * Perhaps "PersistFlightEndpoint" can take an optional >>> "suggested_expiration" timestamp, which the server is free to ignore >>> (some clients may only need to extend the expiration by two minutes, >>> others by two days...) >> >> Interesting. It may be useful for some systems. >> >> Most of existing systems we listed in "Prior Art" (Dremio, >> Google BigQuery Storage and Snowflake) can't specify >> expiration time by users. Only Dremio can specify expiration >> time by users from its admin page: >> https://docs.dremio.com/software/advanced-administration/job-results-cleanup/ >> >> So this may not be needed for now. How about accepting a >> specific request message instead of FlightEndpoint directly >> as "PersistFlightEndpoint" input? >> >> Current: >> PersistFlightEndpoint(FlightEndpoint) >> >> New: >> PersistFlightEndpoint(PersistFlightEndpointRequest) >> >> message PersistFlightEndpointRequest { >> FlightEndpoint endpoint = 1; >> // We may add this later. >> // google.protobuf.Timestamp suggested_expiration_time = 2; >> } >> >> David and Matt, what do you think about this suggestion? Do >> you have any use-case for this feature? >> >>> * Does the client potentially have to call "PersistFlightEndpoint" on >>> each returned endpoint? >> >> Yes. >> >>> Can it pass several endpoints at once? >> >> No. >> >> At first "RefreshFlightEndpoint" was "RefreshFlightInfo" and >> "RefreshFlightInfo" extends all expiration time of >> FlightEndpoints in the given FlightInfo. But the API will be >> difficult to use because some of extend operations may be >> failed in one request. In the case, what should we return? >> Only succeeded FlightEndpoints? How to detect which >> FlightEndpoints are succeeded by clients? >> >> So this propose doesn't provide an API to operate multiple >> FlightEndpoints at once. >> >> See also: >> https://github.com/apache/arrow/issues/35500#issuecomment-1578200076 >> >>> * What is the expected difference between "CancelFlightInfo" and >>> "CloseFlightInfo"? Both seem to have a similar effect, and the exact >>> behaviour will probably be server-dependent anyway ("cancel" and >>> "close" may have meaningful differences when putting/uploading data, >>> not so much when getting/downloading data, IMHO?). >> >> We're discussing this in another e-mail: >> * https://lists.apache.org/thread/nndt2v7w49bcvfmwo0polv7k3srvm5n5 >> * https://lists.apache.org/thread/5c4n40fqn7b0b5t6yz6wbnmz4vsq753q >> >> I'll reply to them for this. >> >> >> Thanks, >> -- >> kou >> >> In <6696613b-5baa-21c2-c29c-f4ec8aa76...@python.org> >> "Re: [DISCUSS][Format][Flight] Result set expiration support" on Wed, >> 21 Jun 2023 15:35:18 +0200, >> Antoine Pitrou <anto...@python.org> wrote: >> >>> >>> Hi Kou, >>> >>> Can we have an actual PR with the proposed gRPC field, method and >>> docstring additions? >>> >>> Regardless, I have some comments and questions: >>> >>> * "RefreshFlightEndpoint" suggests the server will recompute (refresh) >>> * the results; instead I would suggest "PersistFlightEndpoint" >>> >>> * Perhaps "PersistFlightEndpoint" can take an optional >>> * "suggested_expiration" timestamp, which the server is free to ignore >>> * (some clients may only need to extend the expiration by two minutes, >>> * others by two days...) >>> >>> * Does the client potentially have to call "PersistFlightEndpoint" on >>> * each returned endpoint? Can it pass several endpoints at once? >>> >>> * What is the expected difference between "CancelFlightInfo" and >>> * "CloseFlightInfo"? Both seem to have a similar effect, and the exact >>> * behaviour will probably be server-dependent anyway ("cancel" and >>> * "close" may have meaningful differences when putting/uploading data, >>> * not so much when getting/downloading data, IMHO?). >>> >>> Regards >>> >>> Antoine. >>> >>> >>> >>> Le 21/06/2023 à 02:28, Sutou Kouhei a écrit : >>>> Hi, >>>> David provided the Java implementation. Thanks! >>>> If anyone has any comments about this proposal, please share >>>> them. >>>> Thanks,
