-
Notifications
You must be signed in to change notification settings - Fork 218
Separate client and server context #540
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
base: main
Are you sure you want to change the base?
Conversation
|
Note that the diff will show the contents of #539 as well, please take a look at the commit view. |
|
Will the client and server contexts end up looking exactly the same, just with a serializable part and a non-serializable part containing the |
|
Yeah, mostly. I am still tryong to figure out how the pieces fit together as well. The semantics is that the client has some stuff that does not relate to the server (think send priority for example), and is kept private to the client, same goes for server, which may store per request data on how to respond to it (think mqtt response channel). But now i am also thinking that there might be stuff that may need to be transferred to the server and back, and allow the server to make modifications to it, adding another layer pf comminication besides the actual rpc call. Think cookies in the http world, we could effectively extend tarpc with sessions, and with that proper authentication, stateful requests, etc. What do you think? The implementation is not very nice right now, it would be probably better to make the context generic, so that the transport can effectively choose a proper struct instead of an anymap... maybe a trait so that the deadline and trace context can be extracted?... Would need to focus uninterrupted on this for a coiple of hours which is not easy, haha. |
|
Allright, I'm not entirely happy with how this looks yet, but it's a step in the right direction. I'd like to explore making ClientContext and ServerContext more generic, so the implementor of the server can choose the ServerContext freely, and the implementor of the Client could choose ClientContext. On the server side I believe it may not even be necessary, and would be better to have a fixed ServerContext with an anymap. Adding extractors akin to actix would mean that the service implementation would not need to even know how the ServerContext actually looks like, only that a specific value they are interested in can be extracted from it. On the client side it would be more tricky, but I'm a bit unsure if a custom context on that side makes any sense, except perhaps to communicate with the transport itself (things like hey, this is a high priority request, add this header to the request when sending via http). Also I very much dislike and would need to figure out how to avoid all those manual .with, .map_ok mappings on the transports, although implementing the above may just take care of them. Thoughts, @tikue? |
ce423c7 to
15b84e4
Compare
|
Use this to view the correct diff: axos88/tarpc@make-context-ref-mut...axos88:tarpc:separate-client-and-server-context |
841d8e2 to
116c718
Compare
This is part 2 of the solution to #392, and builds on top of #539.
It separates the client and server contexts, extracts the common part. Only the shared context (curently containing the full old context) is transmitted between the client and server.
This allows the Client and Server contexts to evolve separately, and will be used in a follow-up PR to add extensions that can be used as communication and saving state by the transport, hooks and rpc implementations.