[issue1186900] nntplib shouldn't raise generic EOFError
Martin Panter added the comment: NNTPConnectError does still seem a slightly awkward name. I would go for NNTPConnectionError instead, but I’m also happy to put my bikeshed paint away let this patch be applied as is :) Handling of NNTPTemporaryError with a code of 400 is similar to handling of this EOFError. But I guess there is not much you could do with the API unless you made a separate subclass for 400 errors (like all the new EnvironmentError/OSError subclasses), which would be rather severe. My current workaround looks a bit like this: try: [_, info] = nntp.body(...) except NNTPTemporaryError as err: [code, *msg] = err.response.split(maxsplit=1) if code != 400: raise except EOFError: # Or NNTPConnect(ion)Error msg = () else: break # Handle successful response [msg] = msg or (Server shut down connection,) # Handle connection shutdown by server -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Martin Panter added the comment: Some more points: * I suggest adding something like this to the documentation: exception nntplib.NNTPConnect[ion]Error Exception raised when the server unexpectedly shuts down the connection. * The tests should use BytesIO rather than StringIO. Other than that, I think monkey-patching the file attribute in the tests is fine; that’s probably the way I would do it! * The new exception should be added to __all__. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Martin Panter added the comment: PPS: Documentation should probably have the “New in version 3.5” tag as well -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: Thank yo so much, Martin! I will incorporate these changes and add them soon! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Antoine Pitrou added the comment: Indeed, I find the description rather vague and potentially misleading. Connection establishment would usually refer to the TCP connection, but an EOFError is actually a high-level, logical error (it has nothing to do with networking per se: it's probably more of a server failure - or a client bug perhaps :-)). -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: I'ved changed the comment to say Connection closed unexpectedly. -- Added file: http://bugs.python.org/file36264/nntplib_error_v2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Martin Panter added the comment: I could be wrong, but isn’t this error raised when expecting a response from any command, not just during “connection establishment”? Perhaps change the docstring to say something like “Connection closed unexpectedly” instead. -- nosy: +vadmium ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Changes by Jessica McKellar jesst...@mit.edu: -- keywords: +needs review stage: test needed - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Berker Peksag added the comment: diff -r 8f85262fbe8a Lib/nntplib.py --- a/Lib/nntplib.pySun Jul 06 02:24:24 2014 -0400 +++ b/Lib/nntplib.pyThu Jul 10 16:10:38 2014 -0700 @@ -122,6 +122,9 @@ Error in response data pass +class NNTPConnectError(NNTPError): +Error during connection establishment. +pass Could you also document the new exception? (See Doc/library/nntplib.rst and please add a versionadded directive) The pass statement is redundant, but we could keep it to be consistent with rest of the library. @@ -435,7 +438,7 @@ raise NNTPDataError('line too long') if self.debugging 1: print('*get*', repr(line)) -if not line: raise EOFError +if not line: raise NNTPConnectError() if not line: raise NNTPConnectError looks more readable to me. Also, you could add a more descriptive error message. -- nosy: +berker.peksag versions: +Python 3.5 -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Ezio Melotti added the comment: Wouldn't this be backward incompatible? Even if the EOFError that is raised is not documented explicitly, people might be catching it, and switching to a new exception would break their programs. Maybe NNTPConnectError should inherit from EOFError too? -- nosy: +ezio.melotti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: That's a good point. I can add that so the NNTPConnectError can inherit the EOFError -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: Here is an updated patch. -- Added file: http://bugs.python.org/file35941/nntplib_error.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: I am going to fix it so that it raises the NNTPConnectionError rather than update the documentation. -- nosy: +Lita.Cho, jesstess ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Lita Cho added the comment: I have a fix and added some test coverage in order to make sure the NNTFConnectError was being called. However, in the test case, I am monkey patching. If there is a way to do this with mock, I would appreciate the feedback. -- keywords: +patch Added file: http://bugs.python.org/file35922/nntplib_error.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Changes by Mark Lawrence breamore...@yahoo.co.uk: -- nosy: -BreamoreBoy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Changes by Ankur Ankan ankuran...@gmail.com: -- nosy: +Ankur.Ankan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Mark Lawrence breamore...@yahoo.co.uk added the comment: The OP would accept a documentation change if the code's not going to be changed. -- nosy: +BreamoreBoy versions: +Python 3.2 -Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue1186900] nntplib shouldn't raise generic EOFError
Changes by Daniel Diniz aja...@gmail.com: -- stage: - test needed versions: +Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue1186900 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com