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.

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

[C#][Flight] Add support for non-Kestrel Flight server #41347

Closed
qed- opened this issue Apr 23, 2024 · 3 comments
Closed

[C#][Flight] Add support for non-Kestrel Flight server #41347

qed- opened this issue Apr 23, 2024 · 3 comments

Comments

@qed-
Copy link
Contributor

qed- commented Apr 23, 2024

Describe the enhancement requested

Due to class protection levels it is not currently possible to implement a flight server outside of AspNet.Core.

i.e. to start a flight server as a non-Kestrel GRPC server the following code can't be used due to protection level of the protocol and FlightServerImplementation classes.

using Apache.Arrow.Flight.Protocol;
using Apache.Arrow.Flight.Server.Internal;

var flightServer = new FlightServerImpl();
var server = new Server
{
    Services = {  FlightService.BindService(new FlightServerImplementation(flightServer)) },
    Ports =  { new ServerPort("0.0.0.0", 5001, ServerCredentials.Insecure) }   }
};
server.Start();

Component(s)

C#, FlightRPC

CurtHagenlocher pushed a commit that referenced this issue Sep 14, 2024
….net versions (#41348)

### Rationale for this change

With the existing structure it is not possible to create a flight RPC service as a regular GRPC service, outside of AspNet.Core/Kestrel.

This can be supported by changing the protection level of the generated classes and FlightServiceImplementation.cs 

### What changes are included in this PR?

Change protection level from internal to public for generated protocol files and FlightServiceImplementation.cs

### Are these changes tested?

Confirmed that classes are public in the built assembly.

### Are there any user-facing changes?

Generated protocol classes will become visible to end users.

* GitHub Issue: #41347

Authored-by: David Chapman <[email protected]>
Signed-off-by: Curt Hagenlocher <[email protected]>
@CurtHagenlocher CurtHagenlocher added this to the 18.0.0 milestone Sep 14, 2024
@CurtHagenlocher
Copy link
Contributor

Issue resolved by pull request 41348
#41348

@CurtHagenlocher
Copy link
Contributor

@qed-, I should probably have looked at the build output more closely. Do you actually need .NET 4.6.2 support or is .NET 4.7.2 okay? The reason I ask is that there's at least one dependency here which requires netstandard2.0, and the situation with net462 and netstandard2.0 is a bit messy to say the least. If you don't specifically need 462 then targeting 472 probably makes more sense.

@qed-
Copy link
Contributor Author

qed- commented Sep 16, 2024

Hi @CurtHagenlocher , .net 4.7.2 is fine. I'll create a new PR to update it.

qed- added a commit to qed-/arrow that referenced this issue Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants