classification
Title: add 'directory' option to the http.server module
Type: Stage: resolved
Components: Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, matrixise, mdk, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-11-15 22:31 by matrixise, last changed 2017-05-24 07:32 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
chdir-httpserver.diff matrixise, 2016-11-15 22:31 review
28707.patch mdk, 2016-11-15 23:46 review
issue28707.diff mdk, 2016-12-04 15:11 review
issue28707.diff mdk, 2016-12-05 11:06 review
Pull Requests
URL Status Linked Edit
PR 1776 merged matrixise, 2017-05-23 22:57
Messages (11)
msg280898 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2016-11-15 22:31
When we execute the http.server module, the tool will use the current directory (os.getcwd()) but sometimes we would like to specify a directory on the command line. 

With the next patch, I try to fix this missing feature ;-)

Just with python -m http.server -d /tmp

by default the system will use the current directory.
if necessary, I will show an error if the directory does not exist.
msg280899 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-11-15 22:34
The new feature should be documented in Doc/library/http.server.rst, and maybe also Doc/whatsnew/3.7.rst.

Is it possible to write an unit test?
msg280903 - (view) Author: Julien Palard (mdk) * Date: 2016-11-15 23:46
Hi Stéphane,

Your patch is simple and elegant, but I'm asking myself a question about the idea to pass a class instead of an instance to the TCPServer ctor (I know that's not your choice).

If we were able to pass an instance of SimpleHTTPRequestHandler instead of its class, we'd be able to give the `directory` to the handler during the `main()`, instead of using with `chdir` and `getcwd` to pass the information in a kind of hidden/side channel.

I think that relying on `chdir` and `getcwd` to pass a parameter is not the most pretty or most testable way to do so. Also, having an `os.getcwd()` hardcoded in `SimpleHTTPRequestHandler` does not looks right (again, not your fault, it was here before…) typically for testability, but also when used in a concurrent environment, when cwd may be changed by other "coroutines".

I tried to provide a patch fixing all those condiderations, here it is. still, I can't write a patch as KISS as yours, so I'm probably in the wrong way, at least I tried.
msg280906 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2016-11-16 00:32
Julien Palard added the comment:
> If we were able to pass an instance of SimpleHTTPRequestHandler instead of its class, we'd be able to give the `directory` to the handler during the `main()`, instead of using with `chdir` and `getcwd` to pass the information in a kind of hidden/side channel.

You may be able to use functools.partial() to pass an additional
parameter to the request handler constructor.

We use something like that in asyncio for protocols.
msg280916 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-11-16 06:07
+1 for this idea. I use this command everyday in work and every time must cd to the directory is an annoyance.
msg282346 - (view) Author: Julien Palard (mdk) * Date: 2016-12-04 15:11
@Victor you're right, I forgot that partial can also apply keyword arguments. Here is a new patch.
msg294292 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2017-05-23 23:02
Victor,

I have added the documentation, but for the unit test, there is already a test with the SimpleHTTPRequestHandler class where they specify the cwd attribute, because the code of mdk does not change the behavior, I think it's not needed to write a test.

now, if you think we need a test for this specific case (in fact we only override the constructor of the class and specify the directory parameter), I can write a test, but check the code before.

Thank you.
msg294293 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2017-05-23 23:06
and of course, sorry for this big delay.
msg294311 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-05-24 04:15
Stéphane Wirtel added the comment:

and of course, sorry for this big delay.

I am not sure why you are saying that. Nobody was actively awaiting on this
feature, we all have busy scedules. It's ok ;-)
msg294332 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-05-24 07:29
New changeset a17a2f52c4c3b37414da95a152fc8669978c7c83 by Victor Stinner (Stéphane Wirtel) in branch 'master':
bpo-28707: Add the directory parameter to http.server.SimpleHTTPRequestHandler and http.server module (#1776)
https://github.com/python/cpython/commit/a17a2f52c4c3b37414da95a152fc8669978c7c83
msg294334 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-05-24 07:31
Patch merged. Thanks Stéphane and Julien! Especially because writing the unit test was much more painful than I expected, but it's really worth it!
History
Date User Action Args
2017-05-24 07:32:22hayposetstatus: open -> closed
resolution: fixed
stage: resolved
2017-05-24 07:31:53hayposetmessages: + msg294334
2017-05-24 07:29:08hayposetmessages: + msg294332
2017-05-24 04:15:57hayposetmessages: + msg294311
2017-05-23 23:06:44matrixisesetmessages: + msg294293
2017-05-23 23:02:54matrixisesetmessages: + msg294292
2017-05-23 22:57:16matrixisesetpull_requests: + pull_request1856
2016-12-05 11:06:46mdksetfiles: + issue28707.diff
2016-12-04 15:11:02mdksetfiles: + issue28707.diff

messages: + msg282346
2016-11-16 06:07:02xiang.zhangsetnosy: + xiang.zhang
messages: + msg280916
2016-11-16 00:32:40hayposetmessages: + msg280906
2016-11-15 23:46:12mdksetfiles: + 28707.patch
nosy: + mdk
messages: + msg280903

2016-11-15 22:34:15hayposetmessages: + msg280899
2016-11-15 22:32:06matrixisesetnosy: + haypo
2016-11-15 22:31:49matrixisecreate