Roundup Robot added the comment:
New changeset ad64b6a7c0e2 by Serhiy Storchaka in branch 'default':
Issue #21793: BaseHTTPRequestHandler again logs response code as numeric,
https://hg.python.org/cpython/rev/ad64b6a7c0e2
--
___
Python tracker
Serhiy Storchaka added the comment:
Thank you Martin for noticing a logging issue.
--
resolution: - fixed
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Changes by Berker Peksag berker.pek...@gmail.com:
--
stage: commit review - resolved
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Demian Brecht added the comment:
Well, I’m not entirely sure how I came to the conclusion that errors were a
problem (other than not spending enough time walking through it) and of course
you’re right about the coercion handling it just fine. I’ve removed the
explicit conversion in the code
Berker Peksag added the comment:
LGTM. I left a comment for Serhiy on Rietveld.
--
stage: needs patch - commit review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Serhiy Storchaka added the comment:
Does not changing __str__ to decimal representation (and in this case __str__ =
int.__str__ may be better) lost a part of the point of converting HTTP status
codes to enums?
--
___
Python tracker
Demian Brecht added the comment:
On Feb 20, 2015, at 2:50 PM, Serhiy Storchaka rep...@bugs.python.org wrote:
if isinstance(code, HTTPStatus):
code = '%d' % code
That’s what I’m intending on doing. It’s definitely not as contained as
changing the __str__ implementation of HTTPStatus.
Martin Panter added the comment:
I think you will find the error logging was already fine, since it already uses
%d:
127.0.0.1 - - [21/Feb/2015 04:02:06] code 404, message File not found
127.0.0.1 - - [21/Feb/2015 04:02:06] GET /nonexistant HTTP/1.1
HTTPStatus.NOT_FOUND -
The new codes in
Demian Brecht added the comment:
Latest patch should address all comments. It also fixes the same issue in error
logging which wasn’t previously accounted for. The test file has also been
updated with using HTTPStatus where possible rather than hard coded ints. This
is consistent with other
Demian Brecht added the comment:
The updated patch addresses comments which I’d somehow missed previously, but
keeps the log fix to the __str__ implementation of HTTPStatus (using
int.__str__ rather than format()).
Does not changing __str__ to decimal representation (and in this case __str__
Martin Panter added the comment:
One option might be changing log_request() from
self.log_message('%s %s %s',
self.requestline, str(code), str(size))
to
self.log_message('%s %s %s',
self.requestline, format(code), size)
Using str() is redundant with %s, and
Serhiy Storchaka added the comment:
I would write just:
if isinstance(code, HTTPStatus):
code = '%d' % code
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Martin Panter added the comment:
FYI I opened Issue 23442 for a separate regression to do with the
REQUEST_HEADER_FIELDS_TOO_LARGE name
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
I’ve reverted the patch to use the old format. The main reason being that plain
ints can still be used in most cases as values for code, in which case logging
will be inconsistent with cases using the enum.
--
Added file:
Changes by Eli Bendersky eli...@gmail.com:
--
nosy: -eli.bendersky
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Python-bugs-list
Martin Panter added the comment:
New logfix patch looks good. I would have written format(self) or format(self,
'd') instead of '{:d}'.format(self), but that’s no big deal.
--
___
Python tracker rep...@bugs.python.org
Martin Panter added the comment:
If changing the log format, you might also want to update the comment at the
top of Lib/http/server.py:53. It seems the original format was imitating
https://en.wikipedia.org/wiki/Common_Log_Format, except the timestamp is
slightly different.
--
Demian Brecht added the comment:
Thanks for the catch Martin, it definitely wasn't intended. I've added a patch
to fix the issue and add the extra description as suggested by Ethan.
--
Added file: http://bugs.python.org/file38068/issue21793_logfix.patch
Ethan Furman added the comment:
Without having looked at the code I would guess the fix is as simple as
changing a %s to a %d.
Having said that, it would be nice if the name was also in the log -- something
like:
127.0.0.1 - - [08/Feb/2015 05:05:28] GET / HTTP/1.1 200 (OK) -
--
Changes by Serhiy Storchaka storch...@gmail.com:
--
resolution: fixed -
stage: resolved - needs patch
status: closed - open
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Martin Panter added the comment:
Currently the log output includes the new HTTPStatus codes. I don’t care much
for the log output, but perhaps this wasn’t part of the plan? Before:
$ python3.4 -m http.server
Serving HTTP on 0.0.0.0 port 8000 ...
127.0.0.1 - - [08/Feb/2015 05:05:28] GET /
Roundup Robot added the comment:
New changeset edf669b13482 by Serhiy Storchaka in branch 'default':
Issue #21793: Added http.HTTPStatus enums (i.e. HTTPStatus.OK,
https://hg.python.org/cpython/rev/edf669b13482
--
nosy: +python-dev
___
Python tracker
Serhiy Storchaka added the comment:
Thank you for your contribution Demian.
--
resolution: - fixed
stage: commit review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
--
assignee: rhettinger -
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Changes by Serhiy Storchaka storch...@gmail.com:
--
assignee: - serhiy.storchaka
nosy: +serhiy.storchaka
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Demian Brecht added the comment:
Updated patch addressing further reviews
--
Added file: http://bugs.python.org/file37434/issue21793_6.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
Self review/update: removed two errant breakpoints and updated the what's new
section (missed in my previous patch).
--
Added file: http://bugs.python.org/file37420/issue21793_5.patch
___
Python tracker
Demian Brecht added the comment:
Updated patch addressing review comments. Thanks for the review.
--
Added file: http://bugs.python.org/file37368/issue21793_3.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Changes by Demian Brecht demianbre...@gmail.com:
Added file: http://bugs.python.org/file37370/issue21793_4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Demian Brecht added the comment:
Bump (again) for hopeful merge.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Python-bugs-list
Changes by R. David Murray rdmur...@bitdance.com:
--
stage: patch review - commit review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Berker Peksag added the comment:
I left a couple of comments on Rietveld. Thanks for the patch, Demian.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Demian Brecht added the comment:
Removed draft status code, removed S from VARIANTS_
--
Added file: http://bugs.python.org/file36012/issue21793_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Changes by Raymond Hettinger raymond.hettin...@gmail.com:
--
assignee: - rhettinger
nosy: +rhettinger
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Changes by Berker Peksag berker.pek...@gmail.com:
--
nosy: +berker.peksag
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Changes by Martin Panter vadmium...@gmail.com:
--
nosy: +vadmium
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Python-bugs-list
Demian Brecht added the comment:
Bump for a follow-up review or merge
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
___
Demian Brecht added the comment:
New patch attached addressing review comments.
--
Added file: http://bugs.python.org/file35708/issue21793.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
Updated patch with silly missed doc update.
--
Added file: http://bugs.python.org/file35711/issue21793_1.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
Demian Brecht added the comment:
Attached new patch with changes from review and a stab at an update to the docs.
--
Added file: http://bugs.python.org/file35680/refactor_http_status_codes_1.patch
___
Python tracker rep...@bugs.python.org
New submission from Demian Brecht:
This patch is a follow up to an out of scope comment made by R. David Murray in
#20898 (http://bugs.python.org/issue20898#msg213771).
In a nutshell, there is some redundancy between http.client and http.server
insofar as the definition of http status code,
Changes by R. David Murray rdmur...@bitdance.com:
--
nosy: +barry, eli.bendersky, ethan.furman, orsenthil
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
___
Ethan Furman added the comment:
Left comments in reitvald about modifying the Enum used -- let me know if you
have any questions about that.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21793
43 matches
Mail list logo
