ssec-jhu / evolver-ng

Next gen eVolver controller for bioreactor project - wip
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

interface logging handler duplication #82

Closed amitschang closed 2 months ago

amitschang commented 2 months ago

The base interface does some logging setup, but in case there are two things of the same type or name, we can get duplicates of the handler, for example, if MySensor is descendent of BaseInterface

x = MySensor()
y = MySensor()
print(x.logger.handlers)
print(y.logger.handlers)

what we see is:

[<StreamHandler stderr (INFO)>, <StreamHandler stderr (INFO)>]
[<StreamHandler stderr (INFO)>, <StreamHandler stderr (INFO)>]

because a reconfigure creates new instances in a running process (so that the logger sticks around) , this will likely lead to a lot of increasingly duplicate logs.

It's nice to have the logger in there, but I suspect that most cases we'll want to set up the handlers/formatters in root a certain way and have the loggers derive from there, and otherwise they should probably be set explicitly by the user - so the BaseInterface could prob refrain from setting the handler at all.

jamienoss commented 2 months ago

Good catch! This can be handled by adding the appropriate code to remove the logger in BaseInterface.__del__.

Want me to quickly handle it?

jamienoss commented 2 months ago

Actually, I think it's more a question of not adding new handlers after getLogger etc.

amitschang commented 2 months ago

yes, I would advocate for not adding handlers in the base

jamienoss commented 2 months ago

I think it's totally ok to add them in the base, just not if they already exist as desired.

jamienoss commented 2 months ago

Also, the point of the name attr is to differentiate between instances.