Buffer async inputs to the crt #357
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The "clever" workaround to the crt taking a sync stream doesn't work. This effectively reverts the crt binding to the previous state of buffering everything immediately. This unfortunately means that the input stream must be closed before anything can be read. This is not a tenable state for the future. The ideal resolution is for the crt to support reading from an async stream. We might have to contribute that support. There may be other options, like using futures... or sleep... which might be necessary. But please no.
nateprewitt
reviewed
Dec 19, 2024
Comment on lines
+304
to
+308
| # If the body is an async stream.... read it all into memory. This is | ||
| # very unfortunate, but necessary because the CRT doesn't currently | ||
| # have the capability to read async. We will likely have to implment | ||
| # this capability into it ourselves, or implment a thread-based wrapper. | ||
| crt_body = BytesIO(await request.consume_body_async()) |
There was a problem hiding this comment.
I wonder if we can do anything like readinto with a buffer for the CRT to avoid having to do this all at once. I'll reach out to them to see if there's anything quick we can do here.
There was a problem hiding this comment.
I mean we could ostensibly pass them an InputStream that does its own thing, but at the end of the day it's synchronous.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The "clever" workaround to the crt taking a sync stream doesn't work. This effectively reverts the crt binding to the previous state of buffering everything immediately. This unfortunately means that the input stream must be closed before anything can be read. This is not a tenable state for the future.
The ideal resolution is for the crt to support reading from an async stream. We might have to contribute that support. There may be other options, like using futures... or sleep... which might be necessary. But please no.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.