Skip to content
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

Provide logging.config.configParserConfig #60314

Closed
thbach mannequin opened this issue Oct 2, 2012 · 8 comments
Closed

Provide logging.config.configParserConfig #60314

thbach mannequin opened this issue Oct 2, 2012 · 8 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@thbach
Copy link
Mannequin

thbach mannequin commented Oct 2, 2012

BPO 16110
Nosy @vsajip, @bitdancer
Files
  • configParserConfig.patch: patch to provide functionality
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/vsajip'
    closed_at = <Date 2012-10-09.08:06:43.793>
    created_at = <Date 2012-10-02.15:27:11.085>
    labels = ['type-feature', 'library']
    title = 'Provide logging.config.configParserConfig'
    updated_at = <Date 2014-03-10.22:11:26.680>
    user = 'https://bugs.python.org/thbach'

    bugs.python.org fields:

    activity = <Date 2014-03-10.22:11:26.680>
    actor = 'python-dev'
    assignee = 'vinay.sajip'
    closed = True
    closed_date = <Date 2012-10-09.08:06:43.793>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2012-10-02.15:27:11.085>
    creator = 'thbach'
    dependencies = []
    files = ['27386']
    hgrepos = []
    issue_num = 16110
    keywords = ['patch']
    message_count = 8.0
    messages = ['171812', '171827', '171828', '171996', '172015', '172193', '172459', '213103']
    nosy_count = 4.0
    nosy_names = ['vinay.sajip', 'r.david.murray', 'python-dev', 'thbach']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue16110'
    versions = ['Python 3.4']

    @thbach
    Copy link
    Mannequin Author

    thbach mannequin commented Oct 2, 2012

    Currently logging.config provides a fileConfig function which reads a ini-style file via configparser.ConfigParser. I would like to have a function e.g. configParserConfig which accepts a ConfigParser instance and configures logging directly from the settings found in there. The main reasons for this are:

    1. I think it is rather common for an application that provides an interface to configure its logging via an ini file to use this ini file also for further application configuration. With the current implementation the file is read twice and ConfigParser is initialized two times.

    2. Currently it is not idiomatic how to alter an ini-file configuration e.g. by options passed in via command-line. The new function provides a clear solution: create a ConfigParser instance, parse the ini file, alter the configuration and pass it on to logging.config.configParserConfig.

    In fact, the new functionality is easy to achieve by refactoring logging.config a bit (see attached patch).

    @thbach thbach mannequin added the stdlib Python modules in the Lib dir label Oct 2, 2012
    @bitdancer bitdancer added the type-feature A feature request or enhancement label Oct 2, 2012
    @vsajip
    Copy link
    Member

    vsajip commented Oct 2, 2012

    Thanks for the suggestion - I'm sorry, but I'm not inclined to add this. My reasoning is as follows:

    1. I want to encourage usage of the dictConfig() API, as fileConfig() does not cover as much of the logging API as dictConfig() does (for example, Filters). I'd like to minimise the maintenance I have to do for fileConfig()-related code, and would prefer not to add any auxiliary APIs around fileConfig().
    2. The file reading time and ConfigParser instantiation time are not likely to be a performance problem in practice, as logging configuration is an infrequent operation (a one-off operation in most cases).
    3. You can also pass in a file-like object rather than a filename, and in that case, fileConfig() will use readfp() if available. While you might have to seek to the beginning of the file to pass it to another ConfigParser instance, that is likely to be just a pointer adjustment in a buffer rather than actual disk I/O (config files are usually pretty small).

    I hope you will agree. I'll leave the issue as pending for now, and close it in a day or two.

    @vsajip vsajip self-assigned this Oct 2, 2012
    @bitdancer
    Copy link
    Member

    Vinay, you missed one use case in his request: reading the program's configuration, *modifying it* (based on command line args), and then passing it to logging. How would you suggest he handle that use case? Is there an easy way to get from a loaded configuration file to a dictionary for use in dictConfig?

    @thbach
    Copy link
    Mannequin Author

    thbach mannequin commented Oct 4, 2012

    vinay: I understand your preference of dictConfig over fileConfig as maintainer. But as an application developer I want to provide my user an easy way to adjust logging herself. In the end of the day she is the one knowing what has to be logged in which place. Therefor, having something like fileConfig is essential.

    david: The modified configuration can be passed to fileConfig via a StringIO instance.

    The downside is that ConfigParser instances with e.g. 'allow_no_value' set to True are currently difficult to handle – e.g.:

    >>> sys.version
    '3.2.3 (default, Jun 25 2012, 23:10:56) \n[GCC 4.7.1]'
    >>> cp = configparser.ConfigParser(allow_no_value=True)
    >>> cp.read_string('[foo]\nbar')
    >>> buf = io.StringIO()
    >>> cp.write(buf)
    >>> buf.seek(0)
    0
    >>> logging.config.fileConfig(buf)
    ---------------------------------------------------------------------------
    ParsingError                              Traceback (most recent call last)
    <ipython-input-67-0717fe665796> in <module>()
    ----> 1 logging.config.fileConfig(buf)

    /usr/lib/python3.2/logging/config.py in fileConfig(fname, defaults, disable_existing_loggers)
    64 cp = configparser.ConfigParser(defaults)
    65 if hasattr(fname, 'readline'):
    ---> 66 cp.read_file(fname)
    67 else:
    68 cp.read(fname)

    /usr/lib/python3.2/configparser.py in read_file(self, f, source)
    706 except AttributeError:
    707 source = '<???>'
    --> 708 self._read(f, source)
    709
    710 def read_string(self, string, source='<string>'):

    /usr/lib/python3.2/configparser.py in _read(self, fp, fpname)
    1079 # if any parsing errors occurred, raise an exception
    1080 if e:
    -> 1081 raise e
    1082 self._join_multiline_values()
    1083

    ParsingError: Source contains parsing errors: <???>
    [line 2]: 'bar\n'

    Hence, logging.config.fileConfig should at least provide a way to pass in arguments for the ConfigParser initialization. Anyways, I think it is much cleaner to provide a function which gets the configuration directly from the ConfigParser instance.

    @vsajip
    Copy link
    Member

    vsajip commented Oct 4, 2012

    I could consider relaxing the parameters on fileConfig such that instead of accepting just a string or a file-like object, it additionally accepts a ConfigParser instance. More specifically:

    def fileConfig(file_or_fname_or_cp, defaults=None):
        if isinstance(file_or_fname_or_cp, RawConfigParser):
            cp = file_or_filename_or_cp
        else:
            cp = ConfigParser.ConfigParser(defaults)
            if hasattr(cp, 'readfp') and\
                hasattr(file_or_fname_or_cp, 'readline'):
                cp.readfp(file_or_fname_or_cp)
            else:
                cp.read(file_or_fname_or_cp)
    
        formatters = _create_formatters(cp)

    This will only require (in addition to the above) small tweaks to docs
    and tests. It would appear to fit the bill for your use case. Do you agree?

    @thbach
    Copy link
    Mannequin Author

    thbach mannequin commented Oct 6, 2012

    Yeah, the change you suggest sounds reasonable.

    Thanks for reconsidering the case!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 9, 2012

    New changeset ce0d0d052494 by Vinay Sajip in branch 'default':
    Closes bpo-16110: fileConfig now accepts a pre-initialised ConfigParser instance.
    http://hg.python.org/cpython/rev/ce0d0d052494

    @python-dev python-dev mannequin closed this as completed Oct 9, 2012
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 10, 2014

    New changeset 113341605247 by R David Murray in branch 'default':
    whatsnew: logging.fileConfig accepts ConfigParser instances. (bpo-16110)
    http://hg.python.org/cpython/rev/113341605247

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants