Checking headers to determine auto-streaming #1383
Merged
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
Codecov Report
@@ Coverage Diff @@
## master #1383 +/- ##
==========================================
- Coverage 97.28% 94.59% -2.70%
==========================================
Files 67 109 +42
Lines 4235 7655 +3420
==========================================
+ Hits 4120 7241 +3121
- Misses 115 414 +299
Continue to review full report at Codecov.
|
isidentical
reviewed
May 3, 2022
isidentical
reviewed
May 3, 2022
httpie/output/writer.py
Outdated
| @@ -163,7 +164,10 @@ def get_stream_type_and_kwargs( | |||
| if not is_stream and message_type is HTTPResponse: | |||
| # If this is a response, then check the headers for determining | |||
| # auto-streaming. | |||
| is_stream = headers.get('Content-Type') == 'text/event-stream' | |||
| ct_raw = headers.get('Content-Type', None) | |||
There was a problem hiding this comment.
Since we reference this (and text/event-stream) string in different places now, it might make sense to hoist them to the global scope as a constant (just to the top of the file). E.g.:
CONTENT_TYPE_HEADER_NAME = 'Content-Type'
EVENT_STREAM_TYPE = 'text/event-stream'
|
Overall, it looks amazing. Thanks @egleston for the PR (and finding the bug). Beside those 2 minor comments, I wouldn't touch to the HTTP server code but just rather change the test case (which should be easily adaptable): diff --git a/tests/test_stream.py b/tests/test_stream.py
index a8950d2..8df2af2 100644
--- a/tests/test_stream.py
+++ b/tests/test_stream.py
@@ -124,6 +124,10 @@ def test_redirected_stream(httpbin):
['Accept:text/event-stream'],
3
),
+ (
+ ['Accept:text/event-stream; charset=utf-8'],
+ 3
+ ),
(
['Accept:text/plain'],
1
@@ -132,7 +136,7 @@ def test_redirected_stream(httpbin):
def test_auto_streaming(http_server, extras, expected):
env = MockEnvironment()
env.stdout.write = Mock()
http(http_server + '/drip', *extras, env=env)
assert len([
call_arg
for call_arg in env.stdout.write.call_args_listand add a changelog entry! |
Co-authored-by: Batuhan Taskaya <[email protected]>
This reverts commit fe50bc0. Accomplished through additional test case.
* Refactor palette * Modifiers / change static strings to colors * Colors... * Error-based tests * Styling linting Co-authored-by: Jakub Roztocil <[email protected]>
- Highlighting for options (-x, --x) now doesn't strip the prefix (may be whitespace). - Escape sequences are now cross-platform compatible (directly taken by groff/troff [man's renderer]) - Now we check for the section before displaying the man pages. - On MacOS, there is HTTP(n) which is different from our HTTP(1). This used to conflict with it, and we showed the wrong page. Now we specifically ask foir HTTP(1). - Errors that might happen (e.g non executable man command) is now suppressed. So in the worst case (if anything regarding man execution goes wrong), we'll always display the manual. - Docs for man pages. - HTTPie man pages. - Epilog for the man pages (see also) - Auto-generated comments.
* Hide pretty help * Automatic release update warnings. * `httpie cli check-updates` * adapt to the new loglevel construct * Don't make the pie-colors the bold * Apply review feedback. Co-authored-by: Jakub Roztocil <[email protected]>
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 header-check for auto-streaming should parse out the type/sub-type from optional parameters.
FastAPI/Starlette modifies the Content-Type header for SSE responses (actually, for all
text/*media) by adding thecharsetas an "optional parameter":Content-Type: text/event-stream; charset=utf-8