Jens-G commented on PR #3930:
URL: https://github.com/apache/thrift/pull/3930#issuecomment-5854525888
I'd like to have some documentation or an example of how this is meant to be
used. Reading the code, I don't find it intuitive enough.
What would help most:
1. **A section in `lib/netstd/README.md`.** That file is also the README of
the NuGet package, so users will look there. From what I can tell from the
code, it should answer these questions:
- Which constructor turns the per-call mode on? `new
Calculator.Client(httpTransport, new TBinaryProtocol.Factory())` does. `new
Calculator.Client(new TBinaryProtocol(httpTransport))` keeps the shared
buffers, without any warning.
- Which methods use it? The generated methods such as `add()` do. If you
call `send_add()` and `recv_add()` yourself, they use the shared protocols.
- How do you combine it with `TMultiplexedProtocol`, given that netstd
has no protocol factory for it?
- What is shared across calls (the `HttpClient`, its default headers,
`ConnectTimeout`), and what does each call get for itself (the request and
response buffers)?
- Are socket transports affected? As far as I can see, they aren't.
A short code snippet would go a long way.
2. **`///` doc comments on the three new generated constructors.** Choosing
the constructor is the decision that matters, and people make it in
IntelliSense. A sentence or two on each, saying when the per-call path is used,
would do.
3. **Optionally, a small example in the netstd tutorial:** one client over
HTTP, a few calls in flight at once (`Task.WhenAll`), and one close at the end.
Two things to know first: the tutorial's HTTP server does not start on master
at the moment (THRIFT-6374, which has a one-line fix), and no CI job builds
`tutorial/netstd`, so it needs a local build. Please don't build on the
client's `-mc` option; it has a problem of its own (THRIFT-6375).
And two questions, because I'd rather not guess the intent:
- If the `THttpTransport` is wrapped in a `TBufferedTransport` or
`TFramedTransport`, the per-call mode is silently off, because the wrapper is
not an `ITPerCallTransportProvider`. Is that intended, or should it throw?
- What is the five-argument constructor (`inputProtocol, outputProtocol,
transport, inputProtocolFactory, outputProtocolFactory`) for?
🤖 Generated with [Claude Code](https://claude.ai/code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
