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

add example about compression #2623

Merged
merged 10 commits into from
Aug 9, 2024
Merged

Conversation

yanns
Copy link
Collaborator

@yanns yanns commented Feb 28, 2024

Motivation

When I wanted to decompress request bodies, I had some difficulties finding the right way to do it.

Solution

Having an example could help others finding a way to use compression and decompression quickly.

@dayvejones
Copy link
Contributor

Thanks! This is a useful example. Can you show how much overhead is generated by working with and without compression?

@yanns
Copy link
Collaborator Author

yanns commented Feb 29, 2024

Can you show how much overhead is generated by working with and without compression?

Do you mean showing the difference in bytes? in cpu resource? in memory?

@yanns
Copy link
Collaborator Author

yanns commented Feb 29, 2024

I could also add some tests. Other examples do not have tests in general, so I don't know if it's something we should do or not.

@dayvejones
Copy link
Contributor

Yes, of course, this is not a necessity. Just my wondering - is this feature useful or expensive. Any tests.

@yanns
Copy link
Collaborator Author

yanns commented Feb 29, 2024

Yes, of course, this is not a necessity. Just my wondering - is this feature useful or expensive. Any tests.

I don't have any scientific tests. And the results depend a lot on the kind of data you're compressing.
I use compression from a certain threshold so that small payloads are sent without, and large payloads are compressed.

@yanns
Copy link
Collaborator Author

yanns commented Feb 29, 2024

I've added tests about compression and decompression.

Copy link
Collaborator

@mladedav mladedav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me

examples/compression/README.md Outdated Show resolved Hide resolved
examples/compression/Cargo.toml Outdated Show resolved Hide resolved
async fn root(Json(value): Json<Value>) -> Json<Value> {
Json(value)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a visual delimiter here like // ===== TESTS ===== so that it is clearer for the reader that anything bellow is superfluous for the example they were looking for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a marker like that elsewhere? Seems a bit weird to me honestly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, but we also don't have tests in examples so there really never was a need.

But personally, when I skimmed through the code, I suddenly found myself between the #[test] decorated functions and it wasn't as easy for me to visually find where the code ends and the tests begin since.

But I don't mind if you don't like this and you'd rather just have it as it was.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also move the tests to a separated test.rs. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a good idea! 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples/compression/README.md Outdated Show resolved Hide resolved
examples/compression/src/main.rs Outdated Show resolved Hide resolved
async fn root(Json(value): Json<Value>) -> Json<Value> {
Json(value)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a marker like that elsewhere? Seems a bit weird to me honestly.

yanns and others added 3 commits May 2, 2024 21:48
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Copy link
Member

@jplatte jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for taking so long to get back to this. I'll just merge at this point, I've lost all context but it seems pretty clear that this is at least in a good state and if things can still be improved, that may as well happen in another PR.

@jplatte jplatte merged commit c52bf9e into tokio-rs:main Aug 9, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants