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

Allow flushing of NpgsqlReadBuffer.Stream #4122

Merged
merged 2 commits into from Nov 13, 2021

Conversation

aromaa
Copy link
Contributor

@aromaa aromaa commented Nov 9, 2021

Changes the NpgsqlReadBuffer.Stream Flush to be nop to line up with the Stream.Flush{Async} documentation.

Fixes #4123

@vonzshik
Copy link
Contributor

Well, that's a fun one. Definitely wasn't expecting anyone to call Flush on a read-only stream (least of all CryptoStream).
Anyway, could you please open an issue and add a test (you can put one in BugTests), which asserts that both Flush and FlushAsync don't throw?

@roji roji added this to the 5.0.11 milestone Nov 10, 2021
@roji roji added the bug label Nov 10, 2021
@roji roji removed this from the 5.0.11 milestone Nov 10, 2021
@roji roji removed the bug label Nov 10, 2021
@roji roji unassigned aromaa Nov 10, 2021
@aromaa
Copy link
Contributor Author

aromaa commented Nov 10, 2021

This was an interesting one indeed. And never even expected that the documentation mentions that flush should be valid on read-only streams, least that the BCL type relies on this!

@vonzshik vonzshik merged commit dcaf1c3 into npgsql:main Nov 13, 2021
vonzshik pushed a commit that referenced this pull request Nov 13, 2021
vonzshik pushed a commit that referenced this pull request Nov 13, 2021
@vonzshik
Copy link
Contributor

Backported to 6.0.1 via 5cfe425, 5.0.12 via 40302e4.

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.

NpgsqlReadBuffer.Stream throws on Flush
3 participants