-
Notifications
You must be signed in to change notification settings - Fork 39
Better support for non-async scenarios #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Just curious when you think this will be resolved |
Workaround : Change in line 49 So instead of looking for current event loop metrics create its own. |
If you're hitting this scenario then you are likely doing some explicit multi-threading since The solution above (creating a new event loop per logger) can run you into race conditions if you try to create multiple loggers on the same thread. It also has a lot of overhead by creating an event loop for each logger. Rather than doing that, you probably would want to create a single event loop for the app and run your tasks inside of that event loop rather than doing explicit multi-threading (see this example; alternatively you can use |
Is this still planned? It's causing me all kinds of pain trying to use this package with Flask, Gunicorn and Gevent where for reasons I don't fully understand I cannot use Flask's async support. I don't want to be creating event loops per thread, so I'm using this hack to make MetricsLogger synchronous. Fundamentally I don't think it's really okay for a core library like this to require async support, as in Python it's possible to be in execution contexts where you don't have access to the event loop on the main thread. |
If you are using this within a
ThreadPoolExecutor
you may get the following error:A workaround for this is to use
asyncio.run
instead. However, this has some overhead and it would be better if we provided a synchronous mechanism.@metric_scope
can check whether or not an event loop exists, and if not, it can fall back to using a synchronous logger implementation.This will likely be superseded by #21
The text was updated successfully, but these errors were encountered: