Personally, I wouldn't see Kailash's last comment as a minor comment as he says; this is a major problem with that gRFC: deciding between JNI or Java, and outlining the details of how to actually implement an API that would be compatible with the current one is a design decision that needs to be investigated, and fully described by the gRFC.
On Thu, Feb 16, 2017 at 3:17 PM, kailashs via grpc.io < [email protected]> wrote: > Hi Jason, > > Thanks for sending this proposal, I took a look and had some comments: > > 1) As per the proposal, I think that the recommended approach is indeed go > down the path of a java library wrapper. JNI wrapping is another > possibility, but we should first investigate whether the Java wrapper is > not possible before considering it. > 2) IMO the jruby implementation can live in a separate repository and I > would recommend this to start with. In the future, when the wrapped > implementation is stable and passes interop tests we can consider merging > it into the main repo. I would recommend starting this work outside of the > main gRPC repo. The grpc-ecosystem org <http://github.com/grpc-ecosystem> > is a good candidate. This will allow for maximum flexibility. > 3) It would be useful to define some shallow tests that cover the API. > Such tests should be added to the existing Ruby implementation first, and > can then be used to ensure that the jruby implementation conforms. See > https://github.com/grpc/grpc/blob/master/src/python/grpcio_ > tests/tests/unit/_api_test.py for a starting point. I must reiterate that > this type of testing is very shallow and is not sufficient in itself, but a > good addition to the interop tests. > 4) For any gRPC implementation, we need to ensure that the interop tests > pass. See https://github.com/grpc/grpc-java/tree/master/interop-testing, > https://github.com/grpc/grpc/tree/master/src/ruby/pb/test and > https://github.com/grpc/grpc/blob/master/doc/interop-test-descriptions.md. > The jruby implementation needs to pass these in order to be considered > conforming. We recommend re-using the ruby interop test suite with the > jruby wrapper to the extent possible, independent of the java suite (of > course, these can be leveraged as needed). The nice thing is that if the > APIs fully conform, the ruby interop tests should just pass. > 5) Given the difference between Java and C stacks, there may be API > incompatibilities that might not be surmountable, especially in situations > where the C core implementation specifics is directly exposed upward. Such > situations need to be quantified and further down the line, we can decide > how to tackle them and weigh the cost of doing just that vs a full JNI > implementation. > > Some minor comments on the proposal doc itself: > > 1) Please add details on implementation ownership, so that it can be > discussed. Its unclear if this is something that the proposer is signing up > to implement, is that the plan? > > 2) Could you please update the proposal to cover the implementation > approach in more detail based on some of these points. Ie: Location of the > prototype repo, some discussion on the API surfaces that need to be > conformed to and potential areas you have identified as problematic. This > can go hand in hand with a prototype of the java wrapped approach so that > we can tackle issues as they happen. > > > Thanks! > Kailash > > > > > > > On Thursday, February 16, 2017 at 10:36:25 AM UTC-8, Mike Moore wrote: >> >> On Thu, Feb 16, 2017 at 9:08 AM, Jason Lunn <[email protected]> wrote: >> >>> It has probably been a decade since I had a reason to try to use JNI. >>> Does anyone have any experience building Gems using that approach? Would it >>> be the case that there would be one JAR per supported platform ( >>> x64-mingw32, x86_64-linux, universal-darwin, x86-mingw32), or would it >>> be a fat jar that include multiple JNI wrappers? >>> >>> I am not clear on what capabilities of the existing gem are needed to >>> satisfy the dependency of the Google Cloud gem (which is my personal >>> motivation to see this move forward). Maybe Mike Moore knows better if the >>> client side would be enough? >>> >> >> The Google Cloud gems are clients only, and so supporting the client-half >> of GRPC would be sufficient for our use of GRPC. >> >> >>> I think I prefer to see the JRuby support sit alongside the existing >>> Ruby code in the same repository, and tested in a consistent way. This will >>> help avoid breaking behavior from slipping into the C implementation that >>> isn't observed until someone thinks to update the other repo. It should >>> also make it less likely that a gem update is released for the C derived >>> platforms without a corresponding update for the JRuby variant. >>> >>> On Wednesday, February 15, 2017 at 1:30:59 PM UTC-5, [email protected] >>> wrote: >>>> >>>> As has been noted before, I think this would be possible by doing a >>>> pure implementation, wrapping the java client, or with a JNI wrapper over >>>> the C-core (these implementations probably end up relatively different from >>>> C-wrapping grpc-ruby). >>>> >>>> Actually one thing hasn't been noted yet: If only a client-side library >>>> is needed, then perhaps the jruby-platform target could only need to >>>> emulate the client-half of the c-wrapping grpc-ruby (would probably >>>> simplify things). >>>> >>>> As for where the project could live though, one option is it could go >>>> into the same grpc repo as current grpc-ruby. Another is it could be done >>>> entirely as a third party project in its own repo, for which we could give >>>> some guidance. >>>> >>>> On Wednesday, February 15, 2017 at 8:59:31 AM UTC-8, Jason Lunn wrote: >>>>> >>>>> There is a GRFC <https://github.com/grpc/proposal/pull/13> to add >>>>> support for the JRuby <http://jruby.org/> interpreter, which runs on >>>>> the JVM. >>>>> >>>>> Please provide all feedback on this thread. >>>>> >>>> >> -- > You received this message because you are subscribed to the Google Groups " > grpc.io" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/grpc-io. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/grpc-io/042830fb-0dd9-4ddb-b982-28a24ca23720%40googlegroups.com > <https://groups.google.com/d/msgid/grpc-io/042830fb-0dd9-4ddb-b982-28a24ca23720%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/grpc-io. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAEvr0PEHkp1Fzsd5hMc3juyFbxRW77Ds%2BnJAXXi-s7ahAq7WnQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
