Issue 38716: logging: rotating handlers set namer and rotator null

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.

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.

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>
> _______________________________________

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.

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?

> 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.

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>
> _______________________________________

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