log4mongo / log4mongo-python

python logging handler for mongo database
http://log4mongo.org
Other
111 stars 37 forks source link

BufferedMongoHandler (with periodic flushing enabled) can't be initialized outside of main-thread #42

Closed twmr closed 4 years ago

twmr commented 5 years ago

If the BufferedMonogHandler is initialized in a thread that is not alive for the whole runtime of the script, the periodic flushing stops working once the initializer thread is not alive anymore. This is due to the main_thread code in https://github.com/log4mongo/log4mongo-python/blob/master/log4mongo/handlers.py#L251

As a workaround we've removed the main_thread.is_alive() code. Do you have a better solution?

oz123 commented 5 years ago

Can you post a minimal example of how to reproduce this ?

twmr commented 5 years ago

Sure. sth like


import threading
from log4mongo.handlers import BufferedMongoHandler
import logging

logger = logging.getLogger()

def initializer():
     handler = BufferedMongoHandler(**kwargs)
     logger.addHandler(handler)

t = threading(target=initializer)
t.start()
t.join()

# at this point the periodic flush thread is no longer running.

# main code
oz123 commented 5 years ago

To be honest, this is a scenario I didn't think of. Threads in Python are a PitA to manage, so I would avoid them actually. Except when really needed, like in the handler itself. So I must ask, why do you need to stick the handle on a thread?

twmr commented 5 years ago

Our services call bind at the very beginning and then establish connections to other services, which is done in a new thread that is only alive for the initialization-phase. The main thread of each service is responsible for handling RPCs and is not used for establishing network connections.

The reason why we have to call BufferedMongoHandler() in the initializer thread is that its __init__ method establishes (blocking) a connection to the mongodb. If it's possible to change it s.t. a connection is not established in __init__ but in a dedicated method, then we could create the BufferedMongoHandler in our main-thread and establish a connection to the mongodb in the initializer thread.

oz123 commented 5 years ago

I understand now your use case. Feel free to send a PR, we can work a merging of this.

twmr commented 5 years ago

Do you remember in which cases the main_thread check is necessary?

twmr commented 5 years ago

@oz123 What do you think if we remove the main_thread check from the BufferedMongoHandler class?

oz123 commented 5 years ago

@thisch can you send a PR and a few tests, so I can see how this works?

twmr commented 5 years ago

@thisch can you send a PR and a few tests, so I can see how this works?

Sure, my colleague created one, see #43.

oz123 commented 4 years ago

I released a new version to PyPI you might want to consider using it.