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

logging.config.dictConfig will convert namedtuple to ConvertingTuple #83323

Closed
hcoura mannequin opened this issue Dec 27, 2019 · 7 comments
Closed

logging.config.dictConfig will convert namedtuple to ConvertingTuple #83323

hcoura mannequin opened this issue Dec 27, 2019 · 7 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@hcoura
Copy link
Mannequin

hcoura mannequin commented Dec 27, 2019

BPO 39142
Nosy @vsajip, @tirkarthi, @hcoura
PRs
  • bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. #17773
  • [3.8] bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (GH-17773) #17785
  • [3.7] bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (GH-17773) #17786
  • Files
  • bug.py: Script to reproduce the bug
  • 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 = None
    closed_at = <Date 2020-01-01.20:27:28.820>
    created_at = <Date 2019-12-27.20:33:12.469>
    labels = ['3.7', '3.8', '3.9', 'type-crash']
    title = 'logging.config.dictConfig will convert namedtuple to ConvertingTuple'
    updated_at = <Date 2020-01-01.20:27:28.820>
    user = 'https://github.com/hcoura'

    bugs.python.org fields:

    activity = <Date 2020-01-01.20:27:28.820>
    actor = 'vinay.sajip'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-01-01.20:27:28.820>
    closer = 'vinay.sajip'
    components = []
    creation = <Date 2019-12-27.20:33:12.469>
    creator = 'hcoura'
    dependencies = []
    files = ['48806']
    hgrepos = []
    issue_num = 39142
    keywords = ['patch']
    message_count = 7.0
    messages = ['358914', '358920', '358948', '358961', '359166', '359167', '359168']
    nosy_count = 3.0
    nosy_names = ['vinay.sajip', 'xtreak', 'hcoura']
    pr_nums = ['17773', '17785', '17786']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue39142'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @hcoura
    Copy link
    Mannequin Author

    hcoura mannequin commented Dec 27, 2019

    While passing

    {
    "version": 1,
    "disable_existing_loggers": False,
    "formatters": {
    "verbose": {"format": "%(levelname)s %(asctime)s %(module)s %(message)s"}
    },
    "handlers": {
    "stackdriver": {
    "class": "google.cloud.logging.handlers.CloudLoggingHandler",
    "client": client,
    "resource": resource,
    },
    },
    "root": {"level": "INFO", "handlers": ["stackdriver"]},
    }

    to logging.config.dictConfig it will convert resource, which is a namedtuple to ConvertingTuple, this will make google.cloud.logging.handlers.CloudLoggingHandler break down the line.

    I am having to create a wrapper class like

    class Bla:
        resource = logging.resource.Resource(
            type="cloud_run_revision",
            labels={},
        )
    
        def _to_dict(self):
            return self.resource._to_dict()

    to go around this

    @hcoura hcoura mannequin added 3.7 (EOL) end of life type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 27, 2019
    @tirkarthi
    Copy link
    Member

    Thanks for the report. Can you please attach a sample script with the handler used to illustrate the issue?

    @hcoura
    Copy link
    Mannequin Author

    hcoura mannequin commented Dec 28, 2019

    Sorry for not sending a proper reproducible script when submitting the issue. End of the day and quite frustrated with the bug, anyway, I attached a full script that will show the error.

    This is the custom handler I used:

    class MyHandler(logging.StreamHandler):
        def __init__(self, resource, *args, **kwargs):
            super().__init__(*args, **kwargs)
            self.resource: namedtuple = resource
        
        def emit(self, record):
            record.msg += f" {self.resource.type}"
            return super().emit(record)

    self.resource.type will throw and AttributeError when logging, because resource gets converted from a namedtuple to a ConvertingTuple.

    @tirkarthi
    Copy link
    Member

    Thanks for the details. Attached is a complete script. Looking at the source code, anything that looks like a tuple is converted into a tuple. namedtuple produces factory function that itself is a subclass of tuple. It led me to this interesting issue over detecting if it's a namedtuple bpo-7796 where detecting _fields in msg134186 is described as a workaround. So logging might detect namedtuple and skip conversion to ConvertingTuple using the workaround but it appears more of the class of special casing it here in logging that will depend on namedtuple internals. There are no test failures. I will leave it to vinay on this.

    >>> import collections
    >>> person = collections.namedtuple('Person', 'age')
    >>> isinstance(person(age=20), tuple)
    True
    >>> print(person._source) # works on 3.6 but verbose and _source were removed in 3.7 with 8b57d7363916869357848e666d03fa7614c47897
    from builtins import property as _property, tuple as _tuple
    from operator import itemgetter as _itemgetter
    from collections import OrderedDict
    class Person(tuple):
        'Person(age,)'
    // snip more source code for person

    # Detect namedtuple by checking for _fields attribute

    diff --git a/Lib/logging/config.py b/Lib/logging/config.py
    index 4a3b8966ed..ba6937e725 100644
    --- a/Lib/logging/config.py
    +++ b/Lib/logging/config.py
    @@ -448,7 +448,8 @@ class BaseConfigurator(object):
                 value = ConvertingList(value)
                 value.configurator = self
             elif not isinstance(value, ConvertingTuple) and\
    -                 isinstance(value, tuple):
    +                 isinstance(value, tuple) and\
    +                 not hasattr(value, '_fields'):
                 value = ConvertingTuple(value)
                 value.configurator = self
             elif isinstance(value, str): # str for py3k

    @vsajip vsajip added 3.8 only security fixes 3.9 only security fixes labels Dec 31, 2019
    @vsajip
    Copy link
    Member

    vsajip commented Jan 1, 2020

    New changeset 46abfc1 by Vinay Sajip in branch 'master':
    bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (GH-17773)
    46abfc1

    @vsajip
    Copy link
    Member

    vsajip commented Jan 1, 2020

    New changeset 1d5a7e5 by Vinay Sajip (Miss Islington (bot)) in branch '3.8':
    bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (GH-17773) (GH-17785)
    1d5a7e5

    @vsajip
    Copy link
    Member

    vsajip commented Jan 1, 2020

    New changeset 0e0e4ac by Vinay Sajip (Miss Islington (bot)) in branch '3.7':
    bpo-39142: Avoid converting namedtuple instances to ConvertingTuple. (GH-17773) (GH-17786)
    0e0e4ac

    @vsajip vsajip closed this as completed Jan 1, 2020
    @vsajip vsajip closed this as completed Jan 1, 2020
    @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 (EOL) end of life 3.8 only security fixes 3.9 only security fixes type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants