Skip to content
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

codec: add borrow framed to allow temporary codec replacement #5831

Closed
wants to merge 2 commits into from

Conversation

suikammd
Copy link
Contributor

Motivation

Provide a more convenient way of temporarily replacing the codec used by the Framed struct without taking ownership of it.
Currently, the only way to replace the codec in Framed is by using the map_codec method, which consumes the Framed instance and returns a new one with the updated codec. This can be inconvenient if you want to temporarily use a different codec and then switch back to the original one, as you would need to store the original Framed instance somewhere and use map_codec to replace it every time.

Solution

Create a borrowed reference to a Framed instance and use it to construct a new BorrowFramed instance with a different codec through the new added with_codec function. This way, you can use the BorrowFramed instance instead of the original Framed one for as long as you need, and then discard it without affecting the original instance.

@suikammd suikammd force-pushed the master branch 3 times, most recently from 6faea52 to a8d6251 Compare June 28, 2023 11:13
@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-codec Module: tokio-util/codec labels Jun 28, 2023
@Darksonn
Copy link
Contributor

Darksonn commented Jul 5, 2023

This adds a rather large amount of API surface for a feature that I think will be used very rarely. I'm not convinced that it's worth it.

@suikammd
Copy link
Contributor Author

suikammd commented Jul 10, 2023

This adds a rather large amount of API surface for a feature that I think will be used very rarely. I'm not convinced that it's worth it.

Thank you for your feedback on the PR!
To address your concern, I'd like to emphasize that the feature does not affect the existing APIs and provides an optional functionality that users can choose to utilize or not. e.g. We can decode HTTP response using different codecs for header and body.
And if Tokio wants to provide this feature in the future, is there any better ways to solve this problem? I'd be happy to hear them. If not, feel free to close this PR.

@Darksonn
Copy link
Contributor

I'm aware that this only adds new API surface and doesn't modify the existing API.

In general, I find that its usually better to write a single codec that supports your use-case than changing the codec often.

@Darksonn Darksonn closed this Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-codec Module: tokio-util/codec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants