[issue8451] syslog.syslog('msg') logs message with ident python.
Sean Reifschneider j...@tummy.com added the comment: Committed as 80396. Included a change to let openlog(3) pick the ident instead of using the static string python. -- resolution: - accepted status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Sean Reifschneider j...@tummy.com added the comment: Ported to python3 and committed as 80401. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
R. David Murray rdmur...@bitdance.com added the comment: Assuming that it is OK for this to be in 2.7 beta2 (it smells almost like a feature, but I'm certainly willing to let it pass as a bugfix myself), the 2.7 doc change in the commit contains a typo (it says versionchanged 3.2 instead of versionchanged 2.7). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
R. David Murray rdmur...@bitdance.com added the comment: Oh, and you forgot about your note to yourself to update the argument syntax for the 3.2 commit. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
R. David Murray rdmur...@bitdance.com added the comment: One argument in favor of letting openlog pick it (assuming it uses argv[0]) is that for a while at least we will have many systems running both python2 and python3, and it might be useful to have that distinction show up in the log if the fallback is used. I'm not sure this is a strong argument :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Eric Smith e...@trueblade.com added the comment: I have the same reasoning as David, although I was thinking about python vs. pythonw. But it's not a big deal. I think you should check it in as-is, and we can worry about modifying it later, if need be. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Eric Smith e...@trueblade.com added the comment: A couple of points: Didn't we decide that instead of using: openlog(ident[, logopt[, facility]]) we'd use: openlog(ident, logopt=None, facility=None) (or whatever the defaults are)? I can't find a reference, but the argument was that it's how Python signatures are written, so it's clearer if they're all written this way. You should add some comments to syslog_get_argv explaining why you're handling errors the way you are. That is, why you're swallowing exceptions and continuing. Similarly with the call to PyTuple_New(0). I also think it would be clearer if using the string python were inside syslog_get_argv, but that's a style thing. Should the fallback be python, or derived from C's argv[0]? Is it possible that sys.argv[0] would be unicode? Is SEP correct, or should it really be using os.path.sep and/or os.path.altsep? This is probably a nit, but I could see it being a problem under cygwin (which I haven't tested yet). Your if statements shouldn't all be on one line. The single-line style with braces isn't used anywhere else in this module, and it's not in the Python code base that I could see (except for the occasional macro). The example code has some extra spaces around the equal signs. It should be: syslog.openlog(logopt=syslog.LOG_PID, facility=syslog.LOG_MAIL) Thanks for doing this! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
R. David Murray rdmur...@bitdance.com added the comment: The argument documentation style change was made for py3k. The old convention is still used in the 2.x docs. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Sean Reifschneider j...@tummy.com added the comment: Ok, so I left the argument style in the docs as it was. *** NOTE ***: Sean: Change this for the 3 trunk commit. Setting the python string is outside of the argv function because we want to use python on any of the many places where the return is done. As far as using the C argv[0], I decided to specifically use python as a fallback. We could easily just let openlog pick it, which is what the old behavior was, but in that case it's dependent on the implementation. I can't answer about sys.argv being unicode. From my looking at the code, SEP will vary depending on the platform, and is just what os.path.sep is -- I was going to use os.path.sep, but found it just returns SEP. Thanks for the great review, I've attached a new version. -- Added file: http://bugs.python.org/file17018/syslog-kwargs4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Changes by Sean Reifschneider j...@tummy.com: Added file: http://bugs.python.org/file16983/syslog-kwargs2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Antoine Pitrou pit...@free.fr added the comment: Some notes about the patch: - NEWS blurbs generally appear antichronologically, that is newest first :-) - you don't have to mention that The 3.2 changes mentioned above are included in 2.7 - the part of the PyArg_ParseTupleAndKeywords string after the semi-colon is supposed to be a custom error message, which [ident string [, logoption [, facility]]] doesn't look like; how about simply using a colon and putting the function name? - you should check PyList_Size()'s return value for error (sys.argv could have been overriden to something else than a list) - similarly, scriptobj is not guaranteed to be a string object, so you have to check for that too - to check for string length 0, it looks more natural to check either that script[0] != 0, or that PyString_GET_SIZE(...) 0 (rather than calling PyObject_IsTrue()) - why do you Py_INCREF() the openargs tuple? It looks like a reference leak - if PyTuple_New(0) fails (which I admit is highly unlikely), you should free all other resources and bail out - when manually calling syslog_openlog(), you should check the return value and either Py_DECREF it (if non-NULL) or bail out -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Sean Reifschneider j...@tummy.com added the comment: Antoine: I believe I have everything you mentioned addressed with the new patch. That was an awesome review, thank you so much. The only things I didn't do were parts of the last two items you bring up: If PyTuple_New(0) fails, bail out. If syslog_openlog fails, bail out. syslog(3) can continue even if the openlog() fails. It won't have the expected ident string, but it *WILL* log. I believe this is the desired behavior. NOTE: I puled the code out that does all the sys.argv handling, which I think made that whole section of code much easier to read, particularly with the new changes. The down side is that the code to be reviewed is quite different now. I also found a leak in the call to syslog_openlog() where I wasn't DECREFing the return. Can you please review these changes? -- Added file: http://bugs.python.org/file17004/syslog-kwargs3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
New submission from Sean Reifschneider j...@tummy.com:
As discussed in this thread:
http://mail.python.org/pipermail/python-dev/2010-March/098500.html
The syslog module is using the C argv[0] as the program name, not the python
sys.argv[0]. So, in most cases this means that unless you explicitly set a
ident, you get python as the ident. Not entirely helpful.
This patch:
- Makes openlog arguments keyword args.
- Makes openlog ident argument optional.
- If ident is not passed to ident, basename(sys.argv[0]) is used.
- The first call to syslog.syslog() calls ident() with no options
(if it hasn't previously been called).
- Variously related documentation changes.
make test with this succeeds.
I think this is ready to go into the trunk, but would like a review. I'll
check with the release maintainer about if this is appropriate for 2.7b.
Sean
--
components: Library (Lib)
keywords: easy, needs review, patch
messages: 103555
nosy: jafo
priority: normal
severity: normal
stage: patch review
status: open
title: syslog.syslog('msg') logs message with ident python.
type: behavior
versions: Python 2.7, Python 3.2
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8451
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Changes by Sean Reifschneider j...@tummy.com: Added file: http://bugs.python.org/file16982/syslog-kwargs.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8451] syslog.syslog('msg') logs message with ident python.
Changes by Sean Reifschneider j...@tummy.com: -- nosy: +eric.smith ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8451 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
