Add FlightSQL module docs and links to arrow-flight crates
#4012
Conversation
github-actions
bot
added
arrow
alamb
force-pushed
the
alamb/update_docsss
branch
from
4a34d30 to
b3841e1
Compare
alamb
changed the title
arrow-flight crates
| @@ -30,6 +30,11 @@ | |||
| //! | |||
| //! 2. Low level [tonic] generated [`flight_service_client`] and | |||
| //! [`flight_service_server`]. | |||
| //! | |||
| //! 3. Experimental support for [Flight SQL] in [`sql`]. Requires the | |||
There was a problem hiding this comment.
I'd love to make this not experimental BTW. It's just a struct and impl in a library AFAIK, so it shouldn't hurt anyone.
There was a problem hiding this comment.
Yeah, I think we were just trying to give people the heads up that we would likely be making substantial changes to the API -- but then we do that with other parts of arrow, so maybe the extra "experimental" part is unecessary.
What do you think @tustvold / @viirya -- should we rename the feature flag from experimental (and maybe turn it on by default?)
There was a problem hiding this comment.
Where did we get to with an integration test of FlightSQL? I think that would help justify graduating this functionality perhaps?
I'm somewhat apprehensive there may still be areas we have interpreted the specification differently, one was fixed last week. The experimental flag allows us to "fix" such things without needing to preserve backwards compatibility
There was a problem hiding this comment.
The experimental flag allows us to "fix" such things without needing to preserve backwards compatibility
Well, I guess I was thinking we break backwards compatibility fairly regularly (as in there are several API changes per release)
So it isn't like I think the flight sql feature is fully ready and the API won't change -- more like I wonder if we should treat it specially from the rest of arrow-flight
There was a problem hiding this comment.
backwards compatibility fairly
We break Rust API compatibility for sure, protocol compatibility is a different beast imo, as providing an incremental migration story becomes very important for upgrades to actually be possible
There was a problem hiding this comment.
I see -- you are perhaps imagining that the FlightSQL protocol itself may change, not just the rust implementation. The spec is marked as experimental in many places. 🤷
| @@ -15,6 +15,26 @@ | |||
| // specific language governing permissions and limitations | |||
| // under the License. | |||
|
|
|||
| //! Support for execute SQL queries using [Apache Arrow] [Flight SQL]. | |||
| //! | |||
| //! [Flight SQL] is built on top of Arrow Flight RPC framework, by | |||
There was a problem hiding this comment.
This might be easier to read if the link anchors were moved to the end
There was a problem hiding this comment.
Good idea -- done in 7c8f3e1
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
…alamb/update_docsss
Which issue does this PR close?
re #3887
Rationale for this change
While waiting for CI to run on #3887 I spent some time writing docs to make it easier to understand what is going on / what we have in this crate.
What changes are included in this PR?
arrow-flightto point at thesqlfeature.Are there any user-facing changes?
more docs