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.

Sign up for GitHub

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

Jump to bottom

Default FlightSqlService Implementations #4372

Closed
tustvold opened this issue Jun 6, 2023 · 3 comments · Fixed by #4485
Closed

Default FlightSqlService Implementations #4372

tustvold opened this issue Jun 6, 2023 · 3 comments · Fixed by #4485
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers help wanted

Comments

@tustvold
Copy link
Contributor

tustvold commented Jun 6, 2023

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

Noticed whilst working on apache/datafusion#6374, this trait is now substantial and requires a significant amount of boilerplate to implement a service, that may not be interested in a significant portion of the API surface

Describe the solution you'd like

I would like default implementations that return Status::unimplemented for all the trait methods

Describe alternatives you've considered

We could only provide impls for some subset, but this is likely more confusing than just providing blanket implementations for all of them

Additional context

FYI @alamb @avantgardnerio
@tustvold tustvold added enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers arrow-flight Changes to the arrow-flight crate help wanted labels Jun 6, 2023
@alamb
Copy link
Contributor

alamb commented Jun 7, 2023

I think the trait in question is https://docs.rs/arrow-flight/41.0.0/arrow_flight/sql/server/trait.FlightSqlService.html (and it has 40 methods!)

@avantgardnerio
Copy link
Contributor

SGTM.

Another potential cleanup avenue would be exposing higher level abstractions (i.e. the ability to provide an object representing over database structure), then the 20 of these are get_flight_info XXX might be consolidated into one, with less onerous demands on the implementer.

rossjones added a commit to rossjones/arrow-rs that referenced this issue Jun 30, 2023
@rossjones
Add default implementations to the FlightSqlService trait
91b0968 
The trait currently does not have many default implementations, but it
does have a lot of methods.  This PR adds default implementations for
all methods returning Status::unimplemented to fix apache#4372
@rossjones rossjones mentioned this issue Jul 8, 2023
Merged
rossjones added a commit to rossjones/arrow-rs that referenced this issue Jul 8, 2023
@rossjones
Add default implementations to the FlightSqlService trait
7935cf9 
The trait currently does not have many default implementations, but it
does have a lot of methods.  This PR adds default implementations for
all methods returning Status::unimplemented to fix apache#4372
rossjones added a commit to rossjones/arrow-rs that referenced this issue Jul 8, 2023
@rossjones
Add default implementations to the FlightSqlService trait
e78575a 
The trait currently does not have many default implementations, but it
does have a lot of methods.  This PR adds default implementations for
all methods returning Status::unimplemented to fix apache#4372
tustvold pushed a commit that referenced this issue Jul 8, 2023
@rossjones
Add default implementations to the FlightSqlService trait (#4485)
08a57e9 
The trait currently does not have many default implementations, but it
does have a lot of methods.  This PR adds default implementations for
all methods returning Status::unimplemented to fix #4372
@tustvold tustvold added the arrow Changes to the arrow crate label Jul 14, 2023
@tustvold
Copy link
Contributor Author

label_issue.py automatically added labels {'arrow'} from #4485

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate enhancement Any new improvement worthy of a entry in the changelog good first issue Good for newcomers help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add default implementations to the FlightSqlService trait rossjones/arrow-rs
3 participants
@alamb @tustvold @avantgardnerio