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

[Go] arrow library is not compatible with grpc < 1.45 due to use of reflection experimental interface #33734

Closed
chrisirhc opened this issue Jan 18, 2023 · 2 comments
Closed

[Go] arrow library is not compatible with grpc < 1.45 due to use of reflection experimental interface #33734

chrisirhc opened this issue Jan 18, 2023 · 2 comments
Assignees
@chrisirhc
Labels
Component: Go Type: enhancement
Milestone
12.0.0

Comments

@chrisirhc
Copy link
Contributor

Describe the enhancement requested

When attempting to use arrow library in projects with grpc < 1.45, the reflection was added in v1.45.0 via grpc/grpc-go@18564ff .

This is due to a single line that references an experimental interface in grpc.reflection package:

arrow/go/arrow/flight/server.go

Lines 97 to 99 in c8d6110

// ServiceInfoProvider is an interface used to retrieve metadata about the services to expose.
// If reflection is enabled on the server, all the endpoints can be invoked using grpcurl.
reflection.ServiceInfoProvider

The interface is defined as:
https://github.com/grpc/grpc-go/blob/4c776ec01572d55249df309251900554b46adb41/reflection/serverreflection.go#L69-L83

I propose to inline this interface so that the go arrow library can be used in projects with earlier versions of grpc which don't contain this experimental interface. This should maintain the reflection capabilities introduced in 07e7009 but make go arrow library compatible with grpc < 1.45.

Component(s)

Go
@assignUser
Copy link
Member

cc @zeroshade

@github-actions github-actions bot mentioned this issue Jan 18, 2023
Merged
zeroshade pushed a commit that referenced this issue Jan 19, 2023
@chrisirhc
GH-33734: [Go] make compatible with grpc < 1.45 (#33735)
7579dbd 
### Rationale for this change

Define interface from grpc/reflection rather than importing. The interface is marked as experiment, so defining it here is also a step toward improving stability of the go arrow library.

### What changes are included in this PR?

* As above, inlining the interface

### Are these changes tested?

I believe the the change is already tested via:
https://github.com/apache/arrow/blob/c8d6110a26c41966e539e9fa2f5cb8c31dc2f0fe/go/arrow/flight/flight_test.go#L325-L326
And also via confirming via reading https://github.com/grpc/grpc-go/blob/4e4d8288ff4fb99150a526165c767e6fac3bd5ec/Documentation/server-reflection-tutorial.md . However, I have not done any manual testing beyond this.

### Are there any user-facing changes?
No.

### Feedback requested
I'd pasted the code verbatim from: https://github.com/grpc/grpc-go/blob/4c776ec01572d55249df309251900554b46adb41/reflection/serverreflection.go#L69-L83

Should I have pasted only what's necessary and link to that code block instead?

Authored-by: chua <chua@uber.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
@zeroshade zeroshade added this to the 12.0.0 milestone Jan 19, 2023
@zeroshade
Copy link
Member

Issue resolved by pull request 33735
#33735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chrisirhc chrisirhc

Labels
Component: Go Type: enhancement
Projects
None yet
Milestone
12.0.0
Development

No branches or pull requests
3 participants
@chrisirhc @zeroshade @assignUser