Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
bpo-25095: test_httpservers hangs since Python 3.5#5101
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
the-knights-who-say-ni commentedJan 4, 2018
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed thePSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
602642a to789e85cCompare| @@ -0,0 +1,2 @@ | |||
| RequestHandlerLoggingTestCase.test_get now sends a "Connection" header with | |||
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.
We don't require a NEWS entry for this issue, so I just set the "skip news" label. But it would be nice if we convert this into a comment in the test:
self.con.request('GET','/',headers={'Connection':'close'})
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.
Unfortunately as stated in the linked BPO issue, the test hangs because the socket remains open and readable/writable as per HTTP 1.1, the test hangs as it awaits for the socket to terminate.
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.
What I'm saying is that theMisc/NEWS.d/next/Tests/2018-01-27-13-21-39.bpo-25095.s-xRa5.rst file can be deleted (we don't require a NEWS entry for tests unless the patch is big) And we can add a comment to explain why we needed to addheaders={'Connection': 'close'} inLib/test/test_httpservers.py instead.
WildCard65 commentedJul 27, 2018 via email• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ok, I think I understand now, delete the news and add comment in code thatexplains the change reason correct? |
Correct :) |
Ok, I performed the change you asked. |
@vadmium do you have time to take a look at the latest version of the patch? |
Now that I understand the problem, I think adding "Connection: close" is acceptable. Although IMO settingclose_connection directly, or adding "Content-Length: 0" and cleaning up the client side, would be more robust. |
* Add stdio_encoding and stdio_errors fields to _PyCoreConfig.* Add unit tests on stdio_encoding and stdio_errors.
_PyCoreConfig_Read() is now responsible to choose the filesystemencoding and error handler. Using Py_Main(), the encoding is nowchosen even before calling Py_Initialize()._PyCoreConfig.filesystem_encoding is now the reference, instead ofPy_FileSystemDefaultEncoding, for the Python filesystem encoding.Changes:* Add filesystem_encoding and filesystem_errors to _PyCoreConfig* _PyCoreConfig_Read() now reads the locale encoding for the file system encoding.* PyUnicode_EncodeFSDefault() and PyUnicode_DecodeFSDefaultAndSize() now use the interpreter configuration rather than Py_FileSystemDefaultEncoding and Py_FileSystemDefaultEncodeErrors global configuration variables.* Add _Py_SetFileSystemEncoding() and _Py_ClearFileSystemEncoding() private functions to only modify Py_FileSystemDefaultEncoding and Py_FileSystemDefaultEncodeErrors in coreconfig.c.* _Py_CoerceLegacyLocale() now takes an int rather than _PyCoreConfig for the warning.
Call config_init_fs_encoding() if filesystem_errors is not NULL butfilesystem_encoding is NULL.
Py_DecodeLocale() and Py_EncodeLocale() now use the UTF-8 encoding onWindows if Py_LegacyWindowsFSEncodingFlag is zero.pymain_read_conf() now sets Py_LegacyWindowsFSEncodingFlag in itsloop, but restore its value at exit.
Add support for the "surrogatepass" error handler inPyUnicode_DecodeFSDefault() and PyUnicode_EncodeFSDefault()for the UTF-8 encoding.Changes:* _Py_DecodeUTF8Ex() and _Py_EncodeUTF8Ex() now support the surrogatepass error handler (_Py_ERROR_SURROGATEPASS).* _Py_DecodeLocaleEx() and _Py_EncodeLocaleEx() now use the _Py_error_handler enum instead of "int surrogateescape" to pass the error handler. These functions now return -3 if the error handler is unknown.* Add unit tests on _Py_DecodeLocaleEx() and _Py_EncodeLocaleEx() in test_codecs.* Rename get_error_handler() to _Py_GetErrorHandler() and expose it as a private function.* _freeze_importlib doesn't need config.filesystem_errors="strict" workaround anymore.
PYTHONCOERCELOCALE=warn warning is now emitted later and written intosys.stderr, rather than being written into the C stderr stream.
* Py_FileSystemDefaultEncoding and Py_FileSystemDefaultEncodeErrors default value is now NULL: initfsencoding() set them during Python initialization.* Document how Python chooses the filesystem encoding and error handler.* Add an assertion to _PyCoreConfig_Read().
Use the core configuration of the interpreter, ratherthan using global configuration variables. For example, replacePy_QuietFlag with core_config->quiet.
Modify TestPosixSpawn to run Python using -I and -S options.Disable site module to avoid side effects. For example, on Fedora 28,if the HOME environment variable is not set, site._getuserbase()calls pwd.getpwuid() which opens /var/lib/sss/mc/passwd, but thenleaves the file open which makes test_close_file() to fail.
…thongh-9338)Also point to start of tokens in parsing errors.Fixes bpo-34683
…ythonGH-9497)The GlobalLock() call in UpdateDropDescription() was not checked forfailure.https://bugs.python.org/issue34770
Added header 'Content-Length' with value of zero alongside with 'Connection' header.
Been a while but@vadmium I added the header "Content-Length" alongside "Connection" (I kept the "Connection" header/value as it's part of the HTTP 1.1 protocol) |
the-knights-who-say-ni commentedSep 25, 2018
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed thePSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username onbugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue. You cancheck yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
@vadmium@berkerpeksag Closing this PR caused I messed it up, recreated it at#9564 |
Uh oh!
There was an error while loading.Please reload this page.
RequestHandlerLoggingTestCase.test_get now sends a "Connection" header with
the value of "close" as the default state for HTTP/1.1 is to remain open.
https://bugs.python.org/issue25095