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

Set default argument of logging.disable() to logging.CRITICAL #72710

Closed
AlSweigart mannequin opened this issue Oct 24, 2016 · 11 comments
Closed

Set default argument of logging.disable() to logging.CRITICAL #72710

AlSweigart mannequin opened this issue Oct 24, 2016 · 11 comments
Assignees
Labels
3.7 stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@AlSweigart
Copy link
Mannequin

AlSweigart mannequin commented Oct 24, 2016

BPO 28524
Files
  • default_critical.patch: Implementation of the enhancement, including documentation update
  • 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 2016-12-31.11:40:21.053>
    created_at = <Date 2016-10-24.21:45:10.470>
    labels = ['3.7', 'type-feature', 'library']
    title = 'Set default argument of logging.disable() to logging.CRITICAL'
    updated_at = <Date 2021-11-04.14:31:36.706>
    user = 'https://bugs.python.org/AlSweigart'

    bugs.python.org fields:

    activity = <Date 2021-11-04.14:31:36.706>
    actor = 'eryksun'
    assignee = 'vinay.sajip'
    closed = True
    closed_date = <Date 2016-12-31.11:40:21.053>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2016-10-24.21:45:10.470>
    creator = 'Al.Sweigart'
    dependencies = []
    files = ['45208']
    hgrepos = []
    issue_num = 28524
    keywords = ['patch']
    message_count = 11.0
    messages = ['279341', '279346', '279347', '279349', '279350', '279351', '279355', '279357', '279725', '280127', '284384']
    nosy_count = 0.0
    nosy_names = []
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue28524'
    versions = ['Python 3.7']

    @AlSweigart
    Copy link
    Mannequin Author

    AlSweigart mannequin commented Oct 24, 2016

    As a convenience, we could make the default argument for logging.disable()'s lvl argument as logging.CRITICAL. This would make disabling all logging messages:

    logging.disable()

    ...instead of the more verbose:

    logging.disable(logging.CRITICAL)

    This one-line change is backwards compatible.

    @AlSweigart AlSweigart mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 24, 2016
    @zhangyangyu
    Copy link
    Member

    zhangyangyu commented Oct 25, 2016

    Is disabling all logging messages a common need? Maybe other levels are common but we can't know.

    And at least the doc patch needs a versionchanged tag.

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Oct 25, 2016

    Given that a user can add their on levels, I think that having a default would be misleading (having no arguments implies total disabling but with custom levels the default of CRITICAL might not disable all messages).

    @AlSweigart
    Copy link
    Mannequin Author

    AlSweigart mannequin commented Oct 25, 2016

    xiang.zhang: The use case I've found is that I often have logging enabled while writing code, and then want to shut it off once I've finished. This might not be everyone's use case, but this is a backwards-compatible change since it's currently a required argument. If there's a sensible default, I think this is it.

    rhettinger: We could use sys.maxsize instead of logging.CRITICAL to disable any custom logging levels as well if this is a concern.

    @zhangyangyu
    Copy link
    Member

    zhangyangyu commented Oct 25, 2016

    We could use sys.maxsize instead of logging.CRITICAL to disable any custom logging levels as well if this is a concern.

    sys.maxsize is not the upper bound limit of integers in Python. There is no such value in Python3.

    The use case I've found is that I often have logging enabled while writing code, and then want to shut it off once I've finished.

    How about logger.disabled = True. This can work even if there are custom levels. But it's not a documented feature.

    @AlSweigart
    Copy link
    Mannequin Author

    AlSweigart mannequin commented Oct 25, 2016

    How about logger.disabled = True.

    This seems to violate "there should be one and only one way to do it". What happens when logging.disabled is True but then logging.disable(logging.NOTSET) is called? I could see this being a gotcha.

    Since logging.CRITICAL is 50, I think it's reasonable to assume that no one would create a logging level larger than sys.maxsize.

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Oct 25, 2016

    Put me down for -0. I don't think the minor convenience is worth the loss in clarity.

    @AlSweigart
    Copy link
    Mannequin Author

    AlSweigart mannequin commented Oct 25, 2016

    As a general indicator, I did a google search for "logging.disable(logging.XXX)" for the different levels. The number of results passing ERROR, WARN, DEBUG, and INFO are under a couple dozen each. But the number of results for "logging.disable(logging.CRITICAL)" is 3,800.

    It's a rough estimate, but it shows that 95%+ of the time people call logging.disable() they want to disable all logging.

    @vsajip
    Copy link
    Member

    vsajip commented Oct 30, 2016

    The use case I've found is that I often have logging enabled while writing code, and then want to shut it off once I've finished.

    You could do this by having a configuration which is quite verbose while doing development and then less verbose when in production mode. Then if an issue crops up in production, verbosity could be temporarily turned up in the production configuration while diagnosing the issue, then turned down again later, without making code changes. Remember that verbosity can be set at the handler level too, which sometimes gives finer-grained control of logging verbosity.

    @AlSweigart
    Copy link
    Mannequin Author

    AlSweigart mannequin commented Nov 6, 2016

    Setting up different configurations for dev/prod is a bit more complicated than I'd like for most projects. I'd instead just call logging.disable(logging.CRITICAL).

    The entire point of this is just for the convenience of being able to disable logging messages by calling logging.disable() instead of logging.disable(logging.CRITICAL).

    It's a two-line change, backwards compatible, and (imo) a sensible default. You call logging.disable() expecting it to disable logging. You might want to disable a lower level, but as the Google search shows, most people just want to disable all logging period.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 31, 2016

    New changeset 459500606560 by Vinay Sajip in branch 'default':
    Closes bpo-28524: added default level for logging.disable().
    https://hg.python.org/cpython/rev/459500606560

    @python-dev python-dev mannequin closed this as completed Dec 31, 2016
    @ahmedsayeed1982 ahmedsayeed1982 mannequin added expert-subinterpreters 3.8 and removed stdlib Python modules in the Lib dir 3.7 labels Nov 4, 2021
    @eryksun eryksun added stdlib Python modules in the Lib dir 3.7 and removed expert-subinterpreters 3.8 labels Nov 4, 2021
    @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
    3.7 stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants