Update FlightSQL metadata locations, names and docs #4341
Conversation
github-actions
bot
added
arrow
| @@ -1,123 +0,0 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
moved to metadata/mod.rs
| @@ -27,7 +27,7 @@ | |||
| //! 2. Helpers for encoding and decoding FlightSQL messages: [`Any`] and [`Command`] | |||
| //! 3. A [`FlightSqlServiceClient`] for interacting with FlightSQL servers. | |||
| //! 4. A [`FlightSqlService`] to help building FlightSQL servers from [`FlightService`]. | |||
| //! 5. Structures to build responses for FlightSQL metadata APIs: [`SqlInfoList`] | |||
| //! 5. Helpers to build responses for FlightSQL metadata APIs: [`metadata`] | |||
There was a problem hiding this comment.
I think the docs now look more unified and lead people to the metadata crate more easily
|
|
||
| pub use catalogs::GetCatalogsBuilder; | ||
| pub use db_schemas::GetDbSchemasBuilder; | ||
| pub use sql_info::SqlInfoList; |
There was a problem hiding this comment.
As a follow on it would be awesome to make a SqlInfoList builder similarly to the other builders, but I ran out of time today and figured I will do it as a follow on PR
|
@tustvold if possible it would be great to get this PR into the release |
|
CI appears to be failing for this? |
|
Fixing... |
| @@ -252,7 +249,7 @@ impl FlightSqlService for FlightSqlServiceImpl { | |||
| let endpoint = FlightEndpoint::new().with_ticket(ticket); | |||
|
|
|||
| let flight_info = FlightInfo::new() | |||
| .try_with_schema(get_catalogs_schema()) | |||
| .try_with_schema(&query.into_builder().schema()) | |||
There was a problem hiding this comment.
Turning the query into a builder is so beautiful -- it was a great idea @roeap
| /// Builds rows like this: | ||
| /// | ||
| /// * catalog_name: utf8, | ||
| /// * db_schema_name: utf8, | ||
| pub struct GetSchemasBuilder { | ||
| pub struct GetDbSchemasBuilder { |
There was a problem hiding this comment.
renamed to follow the name of the flightSQL request struct (which has a strange Db in it 🤷 )
|
I am going to take the liberty of merging this PR to get it into the release. We can refine the APIs later but I would like to avoid API churn as much as possible (and this is API churn, but on an unreleased API) |
Which issue does this PR close?
This is a follow on to #4296 from @roeap and related to #4295
Rationale for this change
Since we haven't released these APIs yet and now that we have several metadata builders I wanted to put them in a unified location in the flight sql implementation
What changes are included in this PR?
sql::metadatamoduleschema()to methods on the buildersAre there any user-facing changes?