wangfenjin edited a comment on pull request #1386: URL: https://github.com/apache/arrow-rs/pull/1386#issuecomment-1063552825
Thanks @alamb for your kind review, address some of your comments: 1. I'll add a flight-sql feature flag for this 2. Agree that maybe we can make the API more ergonomic, but I need to do more experiments on this (I'm trying to build a more practical flight-sql-server using this), then we will clear what we need. My suggestion is we can design the API as it is, and after we have better/ more simplified design, we can add them into the trait, and make the low level API as a default implementation in the trait, so the user still have chance to override them if they want. It's very important we leave this flexibility to the user. 3. grpcurl may not work for our testing. As I comment in https://github.com/wangfenjin/arrow-datafusion/pull/1 this PR, I use the [arrow-cpp-cli](https://github.com/apache/arrow/blob/master/cpp/src/arrow/flight/sql/test_app_cli.cc) to connect to this server, it helps when we don't have the client implementation, also it makes sure our implementation is compatible with the cpp. We may also need to think about maintain this compatibility in long term. 4. For the documentation thing, I copy some useful comments from cpp implementation. For more detailed documentation like the protocols, I think it's a joint effort with the cpp community, in this repo we can focus on the rust API documentation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
