-
Notifications
You must be signed in to change notification settings - Fork 37
Streaming fetch #28
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
Streaming fetch #28
Conversation
emilk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I assume you have tested it both locally and on web.
It would be really nice with an usage example in example_eframe, but it can be added in a separate PR.
|
|
||
| let mut reader = resp.into_reader(); | ||
| loop { | ||
| let mut buf = vec![0; 2048]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a rational for the 2048 size?
MTU is usually around 1500 bytes, but I'm not sure that really matters here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just rounded up common path MTU to something that seemed reasonable.
| std::thread::Builder::new() | ||
| .name("ehttp".to_owned()) | ||
| .spawn(move || fetch_streaming_blocking(request, on_data)) | ||
| .expect("Failed to spawn ehttp thread"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This expect could be replaced with calling on_data(Err(…))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would require putting the callback in an Rc, but I guess that's fine. Should I make the same change in the non-streaming native fetch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, let it be then.
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
|
You have a failure in |
Co-authored-by: Emil Ernerfeldt <[email protected]>
|
Not sure what to do about the |
|
The |
Add
ehttp::streamingwhen thestreamingfeature is enabled.On the web,
ehttp::streaming::fetchuses wasm-streams to handle the conversion of a Response body to a Rust asyncStream.On native,
ehttp::streaming::fetchusesureqthe same way as blocking fetch, but usinginto_readeron the body instead ofread_to_end.