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

Added support for repeated field query params #15

Merged
merged 1 commit into from
Dec 10, 2017

Conversation

gfecher
Copy link

@gfecher gfecher commented Nov 19, 2017

No description provided.

@Xorlev
Copy link
Owner

Xorlev commented Dec 10, 2017

This looks great! Thanks for the contribution. :)

@Xorlev Xorlev merged commit f66c17b into Xorlev:master Dec 10, 2017
@gfecher
Copy link
Author

gfecher commented Dec 18, 2017

Thank you for creating the project in the first place!
I am evangelising gRPC at my company and to ease migration I will be relying heavily on this.
We're a dropwizard shop and I'm guessing this is exactly why you created this project, too.
I'm planning a number of other improvements, like being able to run the protoc plugin under Windows and server streaming, I'd appreciate it if you could look at my pull requests when they're ready.

@Xorlev
Copy link
Owner

Xorlev commented Dec 19, 2017

Exactly right. Actually, server streaming already works if you check out #14 (though it's a WIP -- I'd wanted to complete client streams before merging). I'm happy to prioritize merging server streaming if it's of use to you now.

@Xorlev
Copy link
Owner

Xorlev commented Dec 19, 2017

I'm planning a number of other improvements, like being able to run the protoc plugin under Windows

I think this should just be a matter of publishing the appropriate artifact name. https://github.com/Xorlev/grpc-jersey/blob/master/build.gradle#L156-L183

@Xorlev
Copy link
Owner

Xorlev commented Jan 1, 2018

@gfecher Streaming (server->client) has been released with 0.2.0. Please file issues if you have any trouble or ideas on improvements. :)

@gfecher
Copy link
Author

gfecher commented Jan 3, 2018

Thanks, it worked a treat.

I have a number of questions (I'm new to gRPC), maybe you can help.

Now I'm trying to get swagger (and swagger-ui) going with the generated resource. Do you have any suggestions? So far I found an incredibly hacky way of injecting in the @Api annotation at runtime so that swagger discover it (based on https://stackoverflow.com/a/30287201/8483863).

My other question is a bit more general: I want the gRPC server exposed as well as the REST interface. For this I created 2 Servers, one built with a normal NettyServerBuilder and the other with an InProcessServerBuilder and added the same gRPC service implementation instance to both.
The stub passed to the generated JerseyResource uses an InProcessChannel for the latter with a directExecutor (I'm guessing threading is handled by the Jersey server itself, so this is fine).
Is this the right way to do it? Or should the stub use a NettyChannelBuilder to connect to the netty-based Server?

Many thanks,
Feso (pronounced Fesho)

@Xorlev
Copy link
Owner

Xorlev commented Jan 4, 2018

  1. Swagger

I'd support a code generation option to emit Swagger annotations. Like the HttpRule extension that the protoc extension leverages, you could add an extension which lets you define the rest of the Swagger metadata. The grpc-gateway project does exactly this.

  1. https://github.com/Xorlev/grpc-jersey#http-and-grpc I have an example for a "dual stack" server. It has a single executor used to process requests from HTTP (via an InProcessChannel to avoid protobuf/HTTP2 overhead) as well as via gRPC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants