-
Notifications
You must be signed in to change notification settings - Fork 436
feat: provide blanket implementations for ClientHandler and ServerHandler traits #609
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
|
@tninesling Nice! A good idea I had a play with macros to see if it could dedupe some of the volume Let me know what you think of 0b54219 It's somewhat vibecoded so let me know if it does the job and if you like it |
Thanks for taking a look @alexhancock. As long as those tests pass, it fixes the issue I ran into. Although, the macros in that commit add a lot of indirection that make it hard to read. I agree with the direction of deduplicating the implementations though. I added some simpler macros in a493962, which are slightly more verbose, but hopefully easier to read and maintain. |
alexhancock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this seems like a good middleground, and cuts down on a lot of the original bloat. Thanks!
|
@tninesling Can you run the formatter: |
a493962 to
64efce8
Compare
64efce8 to
8058c09
Compare
Adds blanket implementations for
Box<T>andArc<T>whenTimplementsClientHandlerorServerHandler.Motivation and Context
When building an MCP service, we may want to wrap it in a smart pointer before passing it into the tower service constructor. In my case, I want to spawn a background thread to do some work using some session state as requests are processed. However, introducing the
Arcresults in a compilation error:How Has This Been Tested?
Added integration tests and confirmed this fixes the compilation error in my project.
Breaking Changes
I believe adding blanket implementations of this kind is typically considered a breaking change, although it should be a no-op for most users.
Types of changes
Checklist
Additional context
I omitted blanket implementations for
&Tand&mut Tdue to the'staticbound on the traits, and I omittedRc<T>due to theSyncbound.