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

Expose SendStream::poll_write and poll_finish methods #1783

Merged
merged 2 commits into from
Mar 17, 2024

Conversation

jean-airoldie
Copy link
Contributor

Closes #1782

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

I'm open to re-exposing named futures. That would provide access to the full breadth of write variations, rather than just the &[u8] case. On the other hand, it's a very good point that the various AsyncWrite impls commit us to supporting this pattern anyway, so we might as well expose it directly.

quinn/src/send_stream.rs Outdated Show resolved Hide resolved
@jean-airoldie
Copy link
Contributor Author

Personally I prefer having access to the polling functions directly because it makes multi-stage futures much simpler to implement.

@Ralith
Copy link
Collaborator

Ralith commented Mar 17, 2024

Figured; been there myself. That can break down if you want multiple futures to concurrently manipulate the same value, but writes require &mut SendStream anyway, so that isn't a concern here.

@jean-airoldie
Copy link
Contributor Author

Cool. I'll do another one for the read polling, if you don't mind.

@Ralith
Copy link
Collaborator

Ralith commented Mar 17, 2024

SGTM. Best to be consistent, after all.

@djc djc merged commit a100fc7 into quinn-rs:main Mar 17, 2024
8 checks passed
@ibigbug
Copy link

ibigbug commented May 16, 2024

is this removed again in the latest release? this one is relying on it https://github.com/hyperium/h3/blob/master/h3-quinn/src/lib.rs#L600

@djc @Ralith

@djc
Copy link
Collaborator

djc commented May 16, 2024

hyperium/h3#238 already ports h3-quinn to quinn 0.11.

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.

Expose poll_write & poll_close
4 participants