classification
Title: logging: file creation options with FileHandler and friends
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Claudiu.Popa, pitrou, python-dev, vinay.sajip
Priority: low Keywords: patch

Created on 2013-07-02 08:41 by pitrou, last changed 2013-11-05 10:06 by vinay.sajip. This issue is now closed.

Files
File name Uploaded Description Edit
logging.patch Claudiu.Popa, 2013-11-03 10:32 review
logging.patch Claudiu.Popa, 2013-11-03 11:19 review
logging.patch Claudiu.Popa, 2013-11-03 11:21 Proper patch review
Messages (11)
msg192181 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-02 08:41
When starting a service as root, you may want your log files to be created as another user (the service may drop privileges later). It would be nice to have a "chown" option to FileHander. For example it could take either a "username" string, or a ("username", "groupname") tuple.

(a similar request may be made for chmod, but umask helps a lot there)

This is quite low-priority :)
msg202010 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-11-03 10:32
Hello. Patch attached.
msg202012 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-11-03 11:19
Added documentation and the chown parameter for the friends of FileHandler. Should tests be added for those classes as well or is it enough to test FileHandler?
msg202141 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-11-04 14:29
This issue had dropped off my radar, thanks for bringing it back into view.

I see that the patch follows Antoine's suggestion of a "chown" parameter, but I'm not sure that's the best way to provide this functionality. What concerns me about a "chown" parameter:

1. It's POSIX only, which makes me not want to add it to the API for all users (even though they don't need to specify it, as it's defaulted). It's not a really common use case, even on POSIX.
2. It makes the argument list for the rotating handlers even more unwieldy than it is at present. While it's not pretty now, I don't want to make it uglier.
3. If other things like this are requested in the future (e.g. "chmod" as mentioned in the original post), then it makes the argument lists longer and longer if this approach is used to satisfy those requirements.

Instead, a mixin which redefined _open would seem to fulfil the requirements, and the same strategy could be used to cater for other requirements without any stdlib changes. Of course it requires the developer to do a little bit more work than having the work done for them in the stdlib, but I think it's reasonable to meet the requirement this way as it's relatively uncommon. I don't mind adding an working example of this to the cookbook, making it even less work for developers to adopt this approach. While the _open method is technically private as it begins with an underscore, I have no problem letting subclasses override it.

Also, ISTM that your patch would fail at the shutil.chown call if the log file didn't already exist, which is a not uncommon scenario.
msg202142 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-11-04 14:35
Sure, thanks for the review, it's really insightful regarding API design. I will try to contribute an example for the cookbook later this day.
msg202157 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-04 19:24
> Instead, a mixin which redefined _open would seem to fulfil the
> requirements, and the same strategy could be used to cater for other
> requirements without any stdlib changes. 

One desireable aspect is to still be able to configure logging using dictConfig(). I don't know if a mixin could enable this.
msg202159 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-11-04 19:48
If you are using dictConfig(), you don't need to specify a class for your handler: you can specify a callable which configures and returns a handler, and the callable could be a function which created a file with appropriate ownership and then returned a FileHandler or subclass thereof which used that file. I can update the cookbook with suitable examples, with and without using a mixin.
msg202160 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-11-04 19:55
> If you are using dictConfig(), you don't need to specify a class for
> your handler: you can specify a callable which configures and returns
> a handler, and the callable could be a function which created a file
> with appropriate ownership and then returned a FileHandler or subclass
> thereof which used that file.

But can I pass the file owner in the config dict?
msg202174 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-11-04 21:11
> But can I pass the file owner in the config dict?

You should be able to; I'll address that in the example.
msg202201 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-11-05 10:03
New changeset 5636366db039 by Vinay Sajip in branch '3.3':
Issue #18345: Added cookbook example illustrating handler customisation.
http://hg.python.org/cpython/rev/5636366db039

New changeset 388cc713ad33 by Vinay Sajip in branch 'default':
Closes #18345: Merged documentation update from 3.3.
http://hg.python.org/cpython/rev/388cc713ad33
msg202202 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-11-05 10:06
When the online docs update, the cookbook entry should appear at

http://docs.python.org/dev/howto/logging-cookbook.html#customising-handlers-with-dictconfig
History
Date User Action Args
2013-11-05 10:06:08vinay.sajipsetmessages: + msg202202
2013-11-05 10:03:34python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg202201

resolution: fixed
stage: resolved
2013-11-04 21:11:23vinay.sajipsetmessages: + msg202174
2013-11-04 19:55:20pitrousetmessages: + msg202160
2013-11-04 19:48:53vinay.sajipsetmessages: + msg202159
2013-11-04 19:24:05pitrousetmessages: + msg202157
2013-11-04 14:35:22Claudiu.Popasetmessages: + msg202142
2013-11-04 14:29:13vinay.sajipsetmessages: + msg202141
2013-11-03 11:21:29Claudiu.Popasetfiles: + logging.patch
2013-11-03 11:19:17Claudiu.Popasetfiles: + logging.patch

messages: + msg202012
2013-11-03 10:32:07Claudiu.Popasetfiles: + logging.patch

nosy: + Claudiu.Popa
messages: + msg202010

keywords: + patch
2013-07-02 08:41:06pitroucreate