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

Refactor request body streaming state handling #2357

Merged
merged 3 commits into from May 27, 2020
Merged

Conversation

tanner0101
Copy link
Member

@tanner0101 tanner0101 commented May 13, 2020

This new state machine ensures that calls to req.body.drain will only ever happen after the previously returned future has completed. This makes it easier to correctly implement streaming file writes. Addresses https://forums.swift.org/t/how-to-use-nonblockingfileio-for-repeated-writes/36206.

This change ensures that streaming requests will be read completely even if a route ignores their body.

@tanner0101 tanner0101 added bug Something isn't working enhancement New feature or request labels May 13, 2020
@mcritz
Copy link
Contributor

mcritz commented May 20, 2020

@tanner0101 Thanks for working on this. Much appreciated.

I assume this will address concerns with https://forums.swift.org/t/how-to-use-nonblockingfileio-for-repeated-writes/36206/16 ?

@tanner0101
Copy link
Member Author

tanner0101 commented May 20, 2020

Yup. That's what triggered this. I just need to write tests to verify the new state machine and then this is good to go.

@tanner0101 tanner0101 marked this pull request as ready for review May 27, 2020 19:30
@tanner0101 tanner0101 added the semver-minor Contains new API label May 27, 2020
@tanner0101 tanner0101 merged commit 77b84cf into master May 27, 2020
@tanner0101 tanner0101 added this to Done in Vapor 4 via automation May 27, 2020
@tanner0101 tanner0101 deleted the tn-body-stream-sm branch May 27, 2020 19:37
@tanner0101
Copy link
Member Author

These changes are now available in 4.7.0

pull bot pushed a commit to scope-demo/vapor that referenced this pull request May 27, 2020
* Refactor request body streaming state handling

* test early exit streaming response

* add body stream tests
@mcritz
Copy link
Contributor

mcritz commented May 28, 2020

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request semver-minor Contains new API
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants