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
Remove ResponseWriter.flush() and explicit_flush #26500
Conversation
The reliable way to detect aliveness of a socket is to check wfile.write(). flush() is either not necessary (in Python 2) or non-functional (in Python 3), and should be removed for clarity. Detailed explaination can be found at: web-platform-tests#24719
Is this still WIP? Or do you want me to take a look as is? |
@Hexcles: It's ready for review :). |
@Hexcles : Code updated. Thank you! |
@@ -4,7 +4,6 @@ def main(request, response): | |||
response.headers.set(b'Content-Type', b'text/event-stream') | |||
response.headers.set(b'Cache-Control', b'no-cache') | |||
|
|||
response.explicit_flush = True |
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.
So it sems lke these tests using explicit_flush
might be broken? Should we replace this with a mechanism that works in the way they expect (or otherwise edit the tests to call write once)?
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.
Yes and no.
They have always been broken in the sense that flush
and explicit_flush
do not do what they expect. But in practice, they seem to only require content to be flushed out before sleep
, and unbuffered write satisfies that, which also explains why no test result changes.
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'm merging this for now. Feel free to open a new issue on this if you think it's important.
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.
LGTM
The most significant change was web-platform-tests/wpt#26500 Change-Id: Ic057ff627d6c0887e8935192609f94b3fd767762 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562226 Commit-Queue: Robert Ma <robertma@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Auto-Submit: Robert Ma <robertma@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#831421}
The most significant change was web-platform-tests/wpt#26500 Change-Id: Ic057ff627d6c0887e8935192609f94b3fd767762 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562226 Commit-Queue: Robert Ma <robertma@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Auto-Submit: Robert Ma <robertma@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#831421} GitOrigin-RevId: 97f4a1f6256d23980f7e8e317ffebfa63e2952a3
The reliable way to detect aliveness of a socket is to check
wfile.write(). flush() is either not necessary (in Python 2)
or non-functional (in Python 3), and should be removed for clarity.
Detailed explaination can be found at:
#24719