[issue23439] Fixed http.client.__all__ and added a test
Berker Peksag added the comment: Committed now. Thanks for the patch, Martin and thanks for the review, Demian. -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Roundup Robot added the comment: New changeset 21b31f5438ae by Berker Peksag in branch '3.4': Issue #23439: Add missing entries to http.client.__all__. https://hg.python.org/cpython/rev/21b31f5438ae New changeset 95533c9edaaf by Berker Peksag in branch 'default': Issue #23439: Add missing entries to http.client.__all__. https://hg.python.org/cpython/rev/95533c9edaaf -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Martin Panter added the comment: Posting v3 patch that changes from a list to a set of expected API names -- Added file: http://bugs.python.org/file38180/http.client-all.v3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Demian Brecht added the comment: Left a super minor comment in Rietveld, but otherwise LGTM. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Martin Panter added the comment: Posting a new patch which explicitly omits HTTPMessage, parse_headers(), and the status codes. Also added and documented the LineTooLong exception. It is already somewhat covered in the test suite. See also Issue 21257 about the status of parse_headers(). -- Added file: http://bugs.python.org/file38140/http.client-all.v2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Martin Panter added the comment: I don’t have a strong opinion about changing __all__ in these cases. I only noticed the potential problem when I went to add a new class to the module, and thought this was common practice. If we leave it as it is, it would be good to add comment in the source code explaining this decision. Also the test case could still be useful to catch future bugs. The currently situation means the status constants are be missing from pydoc help output, and are not available when you do “from http.client import *” in the interactive interpreter. HTTPMessage is a strange class indeed; see Issue 5053. But it is referenced a few times by the documentation, so I originally assumed it should be in __all__. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Demian Brecht added the comment: If we leave it as it is, it would be good to add comment in the source code explaining this decision. I think that __all__ should be left as-is for the time being. Adding some comments around that decision makes sense to me to avoid any future confusion around that. Also the test case could still be useful to catch future bugs. Agreed. I've added a couple minor comments to the review. Thanks for the work on this! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Martin Panter added the comment: Actually, maybe I should add all those status codes as well, like http.client.OK. Will probably require different patches for 3.4 and 3.5. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Changes by Demian Brecht demianbre...@gmail.com: -- nosy: +demian.brecht ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Berker Peksag added the comment: It's not that important in my opinion. Let's keep it simple for now :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Demian Brecht added the comment: The only real question I have is: why? As far as I'm aware, these are implementation details of the http.client module (there's even a comment in HTTPMessage that it might make sense to move the class altogether). As far as the constants go, they're only there for backwards compatibility and http.HTTPStatus should be preferred. In light of that and the fact that they were not previously in __all__, I agree with Berker about keeping this simple for now. On Wed, Feb 11, 2015, 13:57 Berker Peksag rep...@bugs.python.org wrote: Berker Peksag added the comment: It's not that important in my opinion. Let's keep it simple for now :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Changes by Berker Peksag berker.pek...@gmail.com: -- nosy: +berker.peksag stage: - patch review versions: +Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
Changes by Martin Panter vadmium...@gmail.com: -- keywords: +patch Added file: http://bugs.python.org/file38090/http.client-all.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23439] Fixed http.client.__all__ and added a test
New submission from Martin Panter: This patch was split off my patch for Issue 3566, since it should be less controversial. It adds the HTTPMessage class and the parse_headers() function to __all__. I’m not too sure on the status of the parse_headers() function. It is not mentioned in the “http.client” documentation, but is referenced by the “http.server” module’s BaseHTTPRequestHandler.headers entry. Perhaps it should be left unexported? -- components: Library (Lib) messages: 235719 nosy: vadmium priority: normal severity: normal status: open title: Fixed http.client.__all__ and added a test type: behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23439 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
