This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: logging: rotating handlers set namer and rotator null
Type: Stage: resolved
Components: Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: lorb, vinay.sajip
Priority: normal Keywords: patch

Created on 2019-11-06 09:35 by lorb, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17072 merged lorb, 2019-11-06 09:39
Messages (8)
msg356107 - (view) Author: lorb (lorb) * Date: 2019-11-06 09:35
When subclassing the rotating handlers of the logging module and implementing a namer or rotator method they are silently set to None in __init__. This is surprising behaviour and currently the set a specific namer or rotator it is required to create an instance first and than set them.
msg356112 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-11-06 10:35
The namer and rotator attributes are callables, not methods to be overridden. You can certainly do this with methods and set them accordingly:

class MyHandler(BaseRotatingHandler):
    def __init__(self, *args, **kwargs):
        super(MyHandler, self).__init__(*args, **kwargs)
        self.namer = self.my_namer
        self.rotator = self.my_rotator

    def my_namer(self, default_name):
        return default_name  # or whatever you want here

    def my_rotator(self, source, dest):
        os.rename(source, dest)  # or whatever you want here

Having namer and rotator be callables avoids the need to subclass a handler just to override naming and rotating functionality. So, I think this issue should be closed as "not a bug", and the corresponding PR closed. Thanks for your effort, though - I just think you may have misunderstood the intent of the design.
msg356117 - (view) Author: lorb (lorb) * Date: 2019-11-06 10:52
Thanks for the review and response. I don't understand yet why not to
allow the following while keeping the existing functionality. This
could also be achieved by turning the namer and rotator into class
attributes. I recently created a subclass of the timed rotating
handler where it is beneficial for the handler to be able to access to
self to use the rollover time in the naming.

class MyHandler(BaseRotatingHandler):
    def namer(self, default_name):
        return default_name  # or whatever you want here

    def rotator(self, source, dest):
        os.rename(source, dest)  # or whatever you want here

Am Mi., 6. Nov. 2019 um 11:35 Uhr schrieb Vinay Sajip <report@bugs.python.org>:
>
>
> Vinay Sajip <vinay_sajip@yahoo.co.uk> added the comment:
>
> The namer and rotator attributes are callables, not methods to be overridden. You can certainly do this with methods and set them accordingly:
>
> class MyHandler(BaseRotatingHandler):
>     def __init__(self, *args, **kwargs):
>         super(MyHandler, self).__init__(*args, **kwargs)
>         self.namer = self.my_namer
>         self.rotator = self.my_rotator
>
>     def my_namer(self, default_name):
>         return default_name  # or whatever you want here
>
>     def my_rotator(self, source, dest):
>         os.rename(source, dest)  # or whatever you want here
>
> Having namer and rotator be callables avoids the need to subclass a handler just to override naming and rotating functionality. So, I think this issue should be closed as "not a bug", and the corresponding PR closed. Thanks for your effort, though - I just think you may have misunderstood the intent of the design.
>
> ----------
> nosy: +vinay.sajip
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue38716>
> _______________________________________
msg356119 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-11-06 11:14
They can be methods, but don't need to be. In my example, I showed how you can set the namer and rotator attributes to methods which do have access to self. I don't want to encourage the idea that namer and rotator *have* to be methods that have to be overridden. You can do this in your own code, but I don't especially want to encourage that way of doing things - which is why I specifically didn't make them (namer and rotator) methods.
msg356121 - (view) Author: lorb (lorb) * Date: 2019-11-06 11:55
This PR certainly does not turn them into methods. I believe it also
doesn't encourage the idea that they would have to be. All it does is
preventing __init__ from setting them to None in the case they are
defined as methods. Would you consider it more acceptable to just turn
them into class level attributes of BaseRotatingHandler instead of
setting them in init?
msg356140 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-11-06 15:36
> Would you consider it more acceptable to just turn
them into class level attributes of BaseRotatingHandler instead of
setting them in init?

Yes, that would be better.
msg356142 - (view) Author: lorb (lorb) * Date: 2019-11-06 15:45
I have changed the PR accordingly.

Am Mi., 6. Nov. 2019 um 16:36 Uhr schrieb Vinay Sajip <report@bugs.python.org>:
>
>
> Vinay Sajip <vinay_sajip@yahoo.co.uk> added the comment:
>
> > Would you consider it more acceptable to just turn
> them into class level attributes of BaseRotatingHandler instead of
> setting them in init?
>
> Yes, that would be better.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue38716>
> _______________________________________
msg356156 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-11-06 21:22
New changeset 519cb8772a9745b1c7d8218cabcd2f96ceda4d62 by Vinay Sajip (l0rb) in branch 'master':
bpo-38716: stop rotating handlers from setting inherited namer and rotator to None (GH-17072)
https://github.com/python/cpython/commit/519cb8772a9745b1c7d8218cabcd2f96ceda4d62
History
Date User Action Args
2022-04-11 14:59:22adminsetgithub: 82897
2019-11-07 12:46:50vinay.sajipsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-11-06 21:22:03vinay.sajipsetmessages: + msg356156
2019-11-06 15:45:43lorbsetmessages: + msg356142
2019-11-06 15:36:43vinay.sajipsetmessages: + msg356140
2019-11-06 11:55:05lorbsetmessages: + msg356121
2019-11-06 11:14:39vinay.sajipsetmessages: + msg356119
2019-11-06 10:52:19lorbsetmessages: + msg356117
2019-11-06 10:35:10vinay.sajipsetnosy: + vinay.sajip
messages: + msg356112
2019-11-06 09:39:40lorbsetkeywords: + patch
stage: patch review
pull_requests: + pull_request16581
2019-11-06 09:35:12lorbcreate