crossbario / txaio

Utilities to support code that runs unmodified on Twisted and asyncio
MIT License
58 stars 30 forks source link

Allow make_logger() before use_*() #85

Open olafmandel opened 7 years ago

olafmandel commented 7 years ago

Please allow issuing txaio.make_logger() before calling txaio.use_twisted() or txaio.use_asyncio(). Use case: declaring loggers as class-attributes in a library. Then importing this library is currently only possible after first selecting the used backend. But conceptually, imports should not depend on ordering or on having any (seemingly unrelated) settings be done first.

Simple example: the backend must be selected at (1) already, even though it seems that (2) should be soon enough.

# user code
import txaio
txaio.use_asyncio()  # (1)
import spamlib

txaio.use_asyncio()  # (2)
txaio.start_logging()

food = spamlib.Spam()
food.fry()

# spamlib.py
import txaio

class Spam:
    log = txaio.make_logger()  # (3)

    def fry(self):
        self.log.info("Frying spam")

Possible solutions (all allow to move the backend selection from (1) to (2)):

  1. Move (3) to __init__(): this turns the class-attribute into an instance-attribute. Depending on how many instances of the class get created, this may not be what the library author wants.

  2. Make txaio._unframework.make_logger() return a shim that gets modified into the real logger by txaio.start_logging(). For this each shim objects needs to register itself (see e.g. txaio.aio._loggers or txaio.tx._loggers) so make_logger can replace it.

  3. Make txaio._unframework.make_logger() return a shim that replaces itself with the real logger on any call to one of its methods or raises the exception if the backend is still not selected. This seems to be more hacky compared to the above option, but it does not depend on txaio.start_logging() ever getting called.

meejah commented 7 years ago

I'm not sure if this will be possible: txaio is designed around being selected at import time -- in Autobahn, we have "framework-specific" classes that do the txaio.use_*() calls (and also import "generic" classes) so that from a user perspective they either do from autobahn.twisted import X or from autobahn.asyncio import X.

The other part of the design is to avoid whenever possible creating or returning "wrapping" objects. Unfortunately, there are some asyncio cases where we do this: FailuedFuture (because asyncio has nothing like a Failure) and for the logger. There are no wrapped objects for Twisted.

All that said, a use-case like your describe is exactly how the loggers are intended to be used (as class-attributes). Autobahn hasn't run into this requirement yet, as you basically have to import from a framework-specific package.

Since the asyncio logger already has a txaio-provided object involved, it wouldn't be too disruptive to delay it doing the "real thing" until .start_logging got called but the Twisted side already returns a "real" Twisted logger right away.

However, another option is to structure spamlib around the "select framework at import time" philosophy so that e.g. instead of from spamlib import ApiThing you'd do either of from spamlib.twisted import ApiThing or from spamlib.asyncio import ApiThing. This lets your users be blissfully ignorant of txaio's existence entirely -- and they have to select their framework somehow anyway (and forcing them to do it via the imports has certain advantages). This also forces you as the library author to "hide" any common functions or classes behind framework-specific packages (which, again, has certain advantages).

olafmandel commented 7 years ago

The "autobahn way" of having small helper packages autobahn.twisted and autobahn.asyncio that basically derive their classes from the common implementation is certainly interesting. It might actually be what I want in order to hide the fact that txaio is used at all. But will every library author want it like this or is there some merit to allowing the user to select the loop after the import statements?

BTW: start_logging() does not return a raw twisted object (at least for version 2.6.0): it returns a txaio.tx.Logger object that is a thin shim until such a time as start_logging calls set_global_log_level. So both implementations already use wrapping objects.

If it gets decided to not modify make_logger, can this ticket then be considered as a request to extend the documentation: add a small example like...

#spamlib/_common.py:
#...

#spamlib/twisted.py
import txaio
txaio.use_twisted()
from spamlib._common import *

#spamlib/asyncio.py
import txaio
txaio.use_asyncio()
from spamlib._common import *
meejah commented 7 years ago

Ah, yes @olafmandel is right there are wrappers/txaio-provided objects in both the Twisted and asyncio cases. Thanks!

And yes, I agree we should either implement the enhancement you're asking for or fix the documentation to suggest "how to organize your library".

meejah commented 7 years ago

I find myself a little hesitant to special-case logging: since you have to structure your library to have Twisted and asyncio specific imports to use the rest of txaio, it seems way more consistent to have the same behavior for logging as well. It's also impossible to use both at once, so that's not a case to consider either.

The only use-case might be that you have some "actually generic" thing that is identical for both asyncio and Twisted implementations, and doesn't call any txaio.* methods until sometime after import (and you're sure that txaio.use_*() has been called). However this last part is really what the whole "import driven" thing is supposed to force you away from (i.e. you can be sure that you've selected a framework first if you must import via a .twisted.* somewhere along the line).

So, I currently favor fixing the documentation...