PEP 391 ready for review
Full View PEP 391 is, I believe, ready for review. http://www.python.org/dev/peps/pep-0391/ I've also worked up an implementation, though not yet checked it in: it's available as a module "dictconfig.py" with accompanying unit tests in "test_dictconfig.py", at: http://bitbucket.org/vinay.sajip/dictconfig I believe I have incorporated into the PEP comments from people both here and on python-list. There have been comments in favour of the proposal, no objections to the proposal as a whole, and some questions and objections about specific details. I believe I have addressed these with changes to the PEP. Discussion threads on python-dev: http://mail.python.org/pipermail/python-dev/2009-October/092695.html http://mail.python.org/pipermail/python-dev/2009-October/092782.html http://mail.python.org/pipermail/python-dev/2009-October/093062.html And on python-list: http://mail.python.org/pipermail/python-list/2009-October/1223658.html http://mail.python.org/pipermail/python-list/2009-October/1224228.html There hasn't been a *huge* amount of discussion, but as it's a niche topic I suppose that's fair enough. I'm ready to actually integrate these changes in the core logging package. As I understand it, the next step is to put the PEP forward for review, which is why I'm sending this mail. I've already sent an equivalent mail to the PEP editors: David Goodger suggested I incorporate the above links (done) and Brett Cannon suggested I send this to python-dev - also, now, done. Looking forward to the feedback! Regards, Vinay Sajip
I would recommend removing the class keyword and replacing it with the () as you have in the custom examples or replacing () with class so it is uniform across all config options handlers: file: class : logging.handlers.RotatingFileHandler formatter: precise filename: logconfig.log maxBytes: 1024 backupCount: 3 custom: (): my.package.MyHandler alternate: cfg://handlers.file This just strikes me as odd to have to remember for built in handlers I need to use the class keyword and for my custom handlers I need to use (). My preference would be the class keyword vs the odd (), as that is what I am defining, the class to be used for this handler. The ext:// I think should be py:// when defining objects to be access via pythons normal import mechanisms and ext:// for resolving external processes or system calls
Dj Gilcrease <digitalxero <at> gmail.com> writes:
I would recommend removing the class keyword and replacing it with the () as you have in the custom examples or replacing () with class so it is uniform across all config options
[snip]
This just strikes me as odd to have to remember for built in handlers I need to use the class keyword and for my custom handlers I need to use (). My preference would be the class keyword vs the odd (), as that is what I am defining, the class to be used for this handler.
If you are using the built in handlers, you will be using a class - hence the key 'class' is used. If you are using a custom factory, then the system does not force you to use a class - you can use any callable (e.g. a function which constructs an instance and sets some attributes of it before returning it), and the use of '()' indicates that you're not being forced to use a class. Of course you can, since a class is a callable, but you're not restricted to classes here.
The ext:// I think should be py:// when defining objects to be access via pythons normal import mechanisms and ext:// for resolving external processes or system calls
The mechanism I proposed can be extended or changed as you suggest according to the norms of a specific developer environment (e.g. organization policies), but the basic system as I see it doesn't (and shouldn't) care about whether a specific value resolves to an internal (e.g. provided by stdlib) value or an external (e.g. provided by 3rd-party lib) value. The resolution process would be exactly the same in either case. Or perhaps I misunderstood what you meant?
Vinay Sajip wrote:
Full View PEP 391 is, I believe, ready for review.
This is my first reading of this. Comments: nit: I believe "both of these serialization formats allow deserialization of Python dictionaries." should be "... to Python dictionaries." "bespoke configuration methods" I had to look up this mostly non-American British word, which I thought meant 'engaged (to be married, as in spoken for)' to guess that you probably mean the much more obscure 'custom configuration methods'. If so, please say so (in plain American ;-) "The logging.config module will have the following additions:" There is currently only one addition. "Nothing will be returned, though exceptions will be raised " Technically incorrect. "Returns None; exceptions ..." or just "Exceptions ...". The doc for print(), for instance, just says what is does and does not mention the default None return. "particular logger has an attached to it a particular handler." delete 'an' " '()' : 'my.package.customFormatterFactory', " This at first appeared to be a typo. You earlier wrote "the user will need to provide a 'factory' - a callable which is called with a configuration dictionary and which returns the instantiated object. This will be signalled by the factory being made available under the special key '()'. " A string is not a callable. So I thought you perhaps meant " '()' : my.package.customFormatterFactory, " without the quotes, or " '()' : <<my.package.customFormatterFactory>>, ", where <<object type>> is understood to be a place filler for an object of the type specified. Then I see "the specified factory callable will be located using normal import mechanisms" If you stick with this, then "This will be signalled by the factory being made available under the special key '()'." would be clearer as "This will be signalled by an absolute import path to the factory being made available under the special key '()'." However, if the code that constructs the dict passed to dictConfig() has the factory in hand, and possibly no absolute import path, why not let it pass the function directly instead of indirectly. (I am here using 'absolute import path' to mean one that logging can resolve, rather than being only resolvable from the dictConfig caller.) Being able to do so is one of the great features of Python. The code for dictConfig could easily wrap the current import. if not hasattr(factory, '__call__'): factory = __import__(factory) "The '()' also serves as a mnemonic that the corresponding value is a callable." More than just that, parentheses preceeded by an expression *are* a call operator, mapped to '__call__', just as the addition operator '+' is mapped to '__add__'. Great choice to me. ... "If serialization is successful, then dictConfig() will be called to process the resulting dictionary" deserialization Terry Jan Reedy
Terry Reedy wrote:
if not hasattr(factory, '__call__'): factory = __import__(factory)
That won't quite work since the string generally isn't referring to a module, and due to the quirk of __import__ returning the top level module since that is what the interpreter needs to bind to a name as part of a normal import statement. This would need to look more like: if not hasattr(factory, '__call__'): mod_name, dot, attr = factory.rpartition('.') if not dot: raise ValueError("<something meaningful>") module = imputil.import_module(mod_name) factory = getattr(module, attr) Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia ---------------------------------------------------------------
Nick Coghlan wrote:
Terry Reedy wrote:
if not hasattr(factory, '__call__'): factory = __import__(factory)
That won't quite work since the string generally isn't referring to a module, and due to the quirk of __import__ returning the top level module since that is what the interpreter needs to bind to a name as part of a normal import statement.
This would need to look more like:
if not hasattr(factory, '__call__'): mod_name, dot, attr = factory.rpartition('.') if not dot: raise ValueError("<something meaningful>") module = imputil.import_module(mod_name) factory = getattr(module, attr)
s/imputil/importlib/ (Oops...) Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia ---------------------------------------------------------------
participants (4)
-
Dj Gilcrease -
Nick Coghlan -
Terry Reedy -
Vinay Sajip