Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++][Python][FlightRPC] Ensure ::Serialize and ::Deserialize are consistently implemented #32360

Closed
asfimport opened this issue Jul 12, 2022 · 5 comments

Comments

@asfimport
Copy link

Structures like Action don't expose these methods even though ones like FlightInfo do.

Reporter: David Li / @lidavidm
Assignee: Quang Hoang / @quanghgx

PRs and other links:

Note: This issue was originally created as ARROW-17052. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Quang Hoang / @quanghgx:
Hi @lidavidm 

I have a couple of queries that need your help.

  1. Please help me on the list of structures that need to expose serialize/deserialize methods? From scanning of cpp/arrow/flight/types.h and Flight.proto, I got: 

Todo list:

  1. Action  (done, as the first step

  2. ActionType

  3. Criteria

  4. Result

  5. FlightEndpoint

  6. FlightPayload  (corresponds to FlightData)

    Already have serialize/deserialize

    
    BasicAuth
    FlightDescriptor
    Ticket
    FlightInfo
    

    No need to add?

    
    Empty
    PutResult
    Location       (has Parse and a number of methods like ForGrpcTcp) 
    SchemaResult   (has GetSchema and serialized_schema)
    
    1. And parts, like language bindings or existing use cases, for example, that are related to this change? I checked arrow-flight.*-test, however, I worry if I am missing something.

    Sincerely,

@asfimport
Copy link
Author

David Li / @lidavidm:
Thanks for grabbing this!

Empty, Location, and FlightPayload do not need this (FlightPayload is basically internal)

I think SchemaResult should still have it (the format is slightly different between serialized_schema and what SerializeToString would return, since there's the Protobuf metadata). The same goes for PutResult.

@asfimport
Copy link
Author

Quang Hoang / @quanghgx:
Thank you David. Final list:

Action
ActionType
Criteria
FlightEndpoint
PutResult *
Result
SchemaResult 

One more query, for PutResult, is it FlightClient::DoPutResult in arrow/flight/client.h?

Sincerely,

@asfimport
Copy link
Author

David Li / @lidavidm:
I guess PutResult is never exposed directly so don't worry about it then

@asfimport
Copy link
Author

David Li / @lidavidm:
Issue resolved by pull request 13986
#13986

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant