Allow overriding of do_get & export useful macro #2582
Conversation
| @@ -301,92 +312,91 @@ where | |||
| &self, | |||
| request: Request<FlightDescriptor>, | |||
| ) -> Result<Response<FlightInfo>, Status> { | |||
| let any: prost_types::Any = | |||
| let message: prost_types::Any = | |||
There was a problem hiding this comment.
More descriptive name.
arrow-flight/src/sql/server.rs
Outdated
| @@ -61,6 +61,18 @@ pub trait FlightSqlService: | |||
| )) | |||
| } | |||
|
|
|||
| /// Implementors may override to handle additional calls to do_get() | |||
| async fn custom_do_get( | |||
There was a problem hiding this comment.
This is the real change
arrow-flight/src/sql/server.rs
Outdated
| "do_get: The defined request is invalid: {:?}", | ||
| String::from_utf8(request.get_ref().ticket.clone()).unwrap() | ||
| ))) | ||
| self.custom_do_get(request, msg).await |
There was a problem hiding this comment.
This is the main purpose of the PR
|
oh @alamb might be interested since it's arrow |
|
I'm not sure I understand this PR, the issue talks about request proxying which seems unrelated to the changes here? |
I should have done a better job at separating my particular use-case from the idea of extensibility as a concept. In this PR, I would like to make the The main driver of these But this PR is just about making it extensible in general. I think the correct place for the discussion would probably be on a Ballista PR, but unfortunately I can't publish a working one until these arrow changes are released and DataFusion upgrades to them. |
Yes, you're probably right. I think I can achieve the same thing in Ballista without much extra effort if I don't export. I'll back that out. At minimum, it should be done in a another PR with more consideration than "well, that's handy". |
|
Benchmark runs are scheduled for baseline = c692b25 and contender = a685c5f. a685c5f is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2581.
Rationale for this change
Described in the issue.
What changes are included in this PR?
Allowing implementers to override
do_getand exporting a useful protobuf macro.Are there any user-facing changes?
This will empower them to do more, but change no behavior by default.