[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Changes by Ent <pyspl...@gmail.com>: -- status: open -> languishing ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue23255> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Hi, Is it possible for this patch to be reviewed now? Regards, Ent -- ___ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue23255> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Thanks Ned Berker, I can only imagine the amount of work the core devs have to deal with. Hope my patch makes it through in next version. Regards, Ent -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: No I think it's better if you put up a separate patch. That way any questions other reviewers will have, you will be better suited to answer them. Cheers! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: @vadmium: Thanks for the suggestion - I opted for the first one. While I wasn't happy with it being called twice, wasn't getting an idea of how to address it. Am I supposed to include your patch as part of my patch? I am not much knowledgeable about the protocol to follow in such cases, so I haven't included it. -- Added file: http://bugs.python.org/file37960/Feb1stb.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: I have updated the patch such that for any directory, if they have a file with name in index_files, it will be served by default. Also a few tweaks. -- Added file: http://bugs.python.org/file37951/Feb1st.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Thanks for the update! I wasn't expecting this to be such a friendly positive experience. Glad to be proven wrong :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Latest patch with simpler(?) logic? @Demian: This is a tough task! And I appreciate your kind words. I have gone through your comments and I have made a few changes as per your suggestion but I have refrained from a few as well. get_status_type, apply_success_headers, apply_headers The reason I wrote these three methods is that when a new dev wants to try out the library, he shouldn't have to handcode all the boilerplate. It makes no sense to to re-write this part of code for each new implementation. It can always be overwritten when required and it follows existing functionality. But I understand your idea of what should be part of Public API and I have changed the access levels of most of such methods to class methods. I have updated `get_status_type` to be of O(1) (hopefully) and removed HTTPStatusType. I think since it is quite easy to overwrite `apply_headers` `_get_status_type`, the default implementation can be straight up used by anyone with no modification and still yield satisfactory behaviour. As for Doc, I really have no idea how to make it better. Maybe someone else can submit a patch later on? Or if you are fine, I will include your own Doc. Thanks again. -- Added file: http://bugs.python.org/file37882/jan27th.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Based on the comments of many good netizens, I have further updated the patch. -- Added file: http://bugs.python.org/file37849/jan25.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: New patch for review! Let me know if anything is missing. -- Added file: http://bugs.python.org/file37840/jan24th.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: @demian: That's a tall order! :) I would love to use HTTPStatus but for some reason http/__init__.py is devoid of code related to it - https://hg.python.org/cpython/file/31982d70a52a/Lib/http/__init__.py I wasn't sure why this change was made because it like feels a step backwards. Will someone else be reverting it back or should I just include it in this patch or create another one? As for rest of your comments, I will make the necessary changes and put up next patch. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: @demian: If you don't mind, could you please elaborate a bit more on `_resolve_path()` you mentioned in the review/comment? Or maybe link me to the type of behaviour you mentioned? I will accordingly make the changes. As for self.apply_headers, I will see if I can make it more generic. As it stands, I have tried not to make any radical changes in existing logic; This being my first patch and all. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Following is updated patch with * Refactored code with helper functions * Unit Tests * Documentation - Explanation + Examples SimpleHTTPRequestHandler's copyfile has been renamed to copy_file but not shutils'. -- Added file: http://bugs.python.org/file37786/helpers-unittests-docs.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: I am having some issues with replying to comments on review page. It is giving 500 error hence posting replies here. @berker.peksag: Thanks! I will add the methods' documentation examples into the Doc/library/http.server.rst. As well as include a few unit tests. I searched through github and there doesn't seem to be many examples of BaseHTTPRequestHandler copyfile. Still if you feel it is better not to make this change, I will revert it back. I have submitted the contribuer-form on the same day as I submitted this patch. Waiting for it to be accepted. Thanks again. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: @vadmium: My Mistake. It should read file path not file object. (500 error when submitting to review page.) Renaming get_html_or_dir_path to get_path_or_dir for accurate description. Also renaming copyfile to more pythonic copy_file. -- Added file: http://bugs.python.org/file37757/simplehttp-fcn-rnm-doc.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
New submission from Ent: Use of http.server.BaseHTTPRequestHandler for exploratory or simple usage, SimpleHTTPRequestHandler provides a good platform to start on. However, the current code in SimpleHTTPRequestHandler's send_head is tightly coupled together as a single unit. This patch aims to break send_head down into composable parts so that any developer can subclass SimpleHTTPRequestHandler to create one's own HTTPRequestHandler class with their own custom behaviour without having to rewrite a lot of repeated code. For example consider a developer wanting to address two usecases * Use SimpleHTTPRequestHandler, but use index.html from a different directory. * For certain GET urls, call on a specific function to response. * Use custom html page instead of index.html class CustomHTTPRequestHandler(SimpleHTTPRequestHandler): def do_GET(self): if self.path =='/': f = self.home_head() elif self.path in self.custom_paths: f = self.special_head() else: f = self.send_head() # ... # Standard Code As a result of the patch, in above scenario instead of copy-pasting logic for browser redirection, getting object for file or directory and, applying headers upon success; Developer can use methods redirect_browser, get_dir_or_html_dir_path and, apply_headers to reduce the code. Basically, applying DRY principle. Note: Since this is but refactoring of existing code without any new functionality being added, no test cases are provided. -- components: Library (Lib) files: simplehttp.patch keywords: patch messages: 234165 nosy: last-ent priority: normal severity: normal status: open title: SimpleHTTPRequestHandler refactor for more extensible usage. type: enhancement versions: Python 3.4 Added file: http://bugs.python.org/file37736/simplehttp.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23255] SimpleHTTPRequestHandler refactor for more extensible usage.
Ent added the comment: Changing base_files to point @ ['index.htm', 'index.html'] -- Added file: http://bugs.python.org/file37737/simplehttp1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23255 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
