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 Field::chunk #901

Merged
merged 8 commits into from Apr 4, 2022
Merged

Add Field::chunk #901

merged 8 commits into from Apr 4, 2022

Conversation

itstristanb
Copy link
Contributor

@itstristanb itstristanb commented Apr 2, 2022

Added the function chunk for exum::extract's multipart Field to give a clearer option to use rather than try_next.

Motivation

I needed a way to stream large files from a frontend file uploader directly into a file. These files can be 10gb+ in size, and need to be paused at any time. Though this is doable with try_next, I felt that it was clearer from the user's side to have chunk.

Solution

By implementing the chunk function for axum::extract's multipart Field, readability of code and nicer access the underlying multer::Field's chunk funciton is allowed.

Change log

  1. Added chunk function to multipart field
  2. Changed to doc comment for clarification of usage, and changed &self to &mut self
  3. Fixed formatting
  4. Cleaned up nits and correct example to reflex best practices
  5. Removed last unwrap

This fixes not being able to stream data from a multipart directly into a file or other output.
@itstristanb itstristanb changed the title Added chunk function to multipart field Fixing Multipart not being able to stream data to file Apr 2, 2022
@itstristanb itstristanb changed the title Fixing Multipart not being able to stream data to file Fixing Multipart not being able to stream data to file in chunks Apr 2, 2022
Copy link

@rmxbalanque rmxbalanque left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@itstristanb itstristanb changed the title Fixing Multipart not being able to stream data to file in chunks Added clearer method of streaming chunked files Apr 2, 2022
Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

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

Makes sense to add as convenience even though it's the same as the Stream impl.

axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
@davidpdrsn davidpdrsn changed the title Added clearer method of streaming chunked files Add Field::chunk Apr 2, 2022
@itstristanb
Copy link
Contributor Author

@davidpdrsn missed a formatting issue in the initial pr cuasing cli to fail. it is fixed now but needs further approval.

Thanks.

Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

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

A few more nitpicks.

Wanna add a changelog entry as well?

axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
axum/src/extract/multipart.rs Outdated Show resolved Hide resolved
@davidpdrsn
Copy link
Member

I've cleaned up the docs just now so should be good now 😊

I've also noticed that we use unwraps here. Sorry if you saw that and based your original commit on that. I'm gonna remove those unwraps as well.

@davidpdrsn davidpdrsn merged commit d417910 into tokio-rs:main Apr 4, 2022
@davidpdrsn
Copy link
Member

Thanks!

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.

None yet

3 participants