Overall, I think it looks better now, but I have a few nitpicks. The proposal says "The interceptCall method allows developers to modify the call options". What are the semantics of modifying these options, particularly the ones that apply to an entire client, not a single method (channelCredentials and host)? Is the MethodDescriptor object supposed to be modifiable?
The three advanced examples use "StatusBuilder", but it doesn't seem to be defined anywhere. They also only appear to work for unary calls. This should at least be mentioned. In the Retry example, the call is retried for any non-OK status code. There are some nuances that should be considered there, like idempotency and the potential to increase traffic to overwhelmed servers that are timing out. We already have a more complete proposal for handling retries in the core at https://github.com/grpc/proposal/blob/master/A6.md, so I think it may not be the best idea to encourage people to retry calls with a relatively simple implementation at a higher level. Also, the Caching example appears to use "savedMessage" and "storedMessage" interchangeably, and it does the same with "savedListener" and "storedListener". And the proposal should say "Implemented in: Javascript" at the top, instead of "Java, Go". On Thu, Mar 9, 2017 at 10:58 AM dduke via grpc.io <[email protected]> wrote: The proposal and the PR are now updated with the new API: https://github.com/grpc/proposal/pull/14 https://github.com/grpc/grpc/pull/9842 On Friday, February 24, 2017 at 4:16:20 PM UTC-8, [email protected] wrote: Looking at this more closely, I think a more flexible outbound/inbound interceptor API will cover the use cases I mentioned without a separate batch-handling API. I'll work on updating the proposal. On Friday, February 24, 2017 at 9:25:13 AM UTC-8, [email protected] wrote: I agree with hiding the grpc.Call API. I'm concerned about the usability of the inbound/outbound interceptor API for use cases like caching. When we need to change how operations are batched or we want to short circuit the remaining interceptor stack, a purely operation-specific API is awkward and difficult to reason about. Would you consider an abstraction over the grpc.Call API like this? { interceptCall: function(callConstructor, options) { var call = callConstructor(options); return (new InterceptingCallBuilder(call)).withInterceptBatch(function(batch, next, responder) { var cachedResponse = _getCachedResponse(batch); if (cachedResponse) { var response = (new ResponseBuilder).withMessage(cachedResponse).withStatus(grpc.status.OK).build(); responder(null, response); } else { next(batch); } }).build(); } } `batch` in this example would wrap the internal client_batch with some accessor methods: `hasMetadata`, `getMetadata`, etc. The returned call's `interceptBatch` would have access to (and the ability to mutate) the full batch and could short circuit the remaining interceptor stack without exposing the internal API. On Thursday, February 23, 2017 at 1:20:38 PM UTC-8, Michael Lumish wrote: I do not like the idea of directly exposing the internal grpc.Call API. It is internal for a reason, and it is not currently guaranteed to be stable. I would strongly prefer that the "outbound interceptor" and "inbound interceptor" APIs be the fundamental building blocks of this interceptor API. On Thursday, February 23, 2017 at 1:01:45 PM UTC-8, Michael Lumish wrote: A proposal for a client interceptor API for the Node.js library has been created here: https://github.com/grpc/proposal/pull/14. Please keep discussion about it 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/878bc96d-f6a7-4504-b106-6654b70de7d4%40googlegroups.com <https://groups.google.com/d/msgid/grpc-io/878bc96d-f6a7-4504-b106-6654b70de7d4%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/CAPK2-4fbXemN9n5s%2Bzi60q8_tk4zyspzKVwzAm4afFeegscvGw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
smime.p7s
Description: S/MIME Cryptographic Signature
