bpo-37857: Invalidate cache when log level changed directly by zaneb · Pull Request #15286 · python/cpython (original) (raw)

I'm not sure if this is needed or not, but there are a couple things I don't like about the approach. Primarily because it's inconstant. I would suggest changing all level references to _level and leave self.manager._clear_cache() in setLevel() since that is the documented interface.

Since Logger.level is not documented, simply switching to Logger._level will not break the published API. However, those accessing Logger.level would not get a failure, they would simply create a new attribute called level and continue to operate at the previous level. This pretty similar to the state we have now.

I think a reasonable compromise might be to change self.level to self._level and make level a read-only property. That would raise AttributeError: can't set attribute when someone tried to set it directly. The traceback should make it obvious this is referring to level. @vsajip, does this seem workable? If this isn't a sufficient error, a non-functioning setter could be introduced that raises a more descriptive exception.

One could argue a deprecation period is needed and a setter could be used to do that. I wouldn't object to that, but I also would argue it isn't needed since it's not documented and already doesn't work in 3.7+. If you did want to introduce a functioning setter for level, it should be independent of the rest of the code, i.e. if you delete it, everything else still works. Basically, the setter would simply call setLevel().