classification
Title: getLogger does not check its argument
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, flox, python-dev, vinay.sajip
Priority: normal Keywords: patch

Created on 2011-11-06 23:46 by flox, last changed 2011-11-07 20:12 by eric.snow. This issue is now closed.

Files
File name Uploaded Description Edit
issue13361_check.diff flox, 2011-11-07 08:53 review
issue13361_dont_check.diff flox, 2011-11-07 08:56
Messages (7)
msg147196 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-06 23:46
>>> import logging
>>> log = logging.getLogger(any)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "./Lib/logging/__init__.py", line 1730, in getLogger
    return Logger.manager.getLogger(name)
  File "./Lib/logging/__init__.py", line 1114, in getLogger
    self._fixupParents(rv)
  File "./Lib/logging/__init__.py", line 1142, in _fixupParents
    i = name.rfind(".")
AttributeError: 'builtin_function_or_method' object has no attribute 'rfind'
>>> log = logging.getLogger(any)
>>> log.error("It should not create this logger")
It should not create this logger
>>>
msg147211 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-11-07 08:55
New changeset 8c719e106694 by Vinay Sajip in branch 'default':
Merged fix for #13361 from 3.2.
http://hg.python.org/cpython/rev/8c719e106694
msg147212 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-07 08:56
I've uploaded two proposals:
 - first with isinstance(name, str)
 - second which is more duck-friendly

Personally, I like ducks.
msg147213 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2011-11-07 09:00
btw, changeset a3ba905447ba does not fix the case for:

import logging
log = logging.Logger(any)
msg147214 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-11-07 09:12
@Florent: Sorry, I didn't see your patch, for some reason. But I would say:

1. I agree that where I put the check (logging.getLogger) does not catch the case where someone instantiates the logger directly (using logging.Logger(any)), but users aren't supposed to instantiate loggers directly anyway - this would not result in a working logger. The check is in the same place where (in 2.7) we check for Unicode and encode to bytes.

2. I don't want to be too liberal in accepting logger names, since they are intended to mean "a place in the application". So, accepting anything other than text does not seem right to me - so str for 3.x, str or unicode for 2.x.

3. I thought a single test (passing in a invalid type) would be sufficient for the logging code, ISTM adding tests with lots of types is actually testing isinstance ;-)

4. I didn't notice your patch, and hence goofed in raising a ValueError instead of (correctly as you had it) a TypeError. I will rectify this.
msg147216 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-11-07 10:16
New changeset 60dd1568bbd1 by Vinay Sajip in branch '2.7':
Closes #13361: Raise correct exception type.
http://hg.python.org/cpython/rev/60dd1568bbd1

New changeset bc05c11b340e by Vinay Sajip in branch '3.2':
Closes #13361: Raise correct exception type.
http://hg.python.org/cpython/rev/bc05c11b340e

New changeset fb73fe5d0ab1 by Vinay Sajip in branch 'default':
Closes #13361: Merge fix from 3.2.
http://hg.python.org/cpython/rev/fb73fe5d0ab1
msg147254 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-11-07 20:12
FYI, the following changesets were also for this issue.  They had the wrong issue number (#13661, which doesn't actually exist so no big deal), which is why they didn't show up in this issue automatically.


New changeset 5f3b7528b144 by Vinay Sajip in branch '2.7':
Closes #13661: Check added for type of logger name.
http://hg.python.org/cpython/rev/5f3b7528b144

New changeset a3ba905447ba by Vinay Sajip in branch '3.2':
Closes #13661: Check added for type of logger name.
http://hg.python.org/cpython/rev/a3ba905447ba
History
Date User Action Args
2011-11-07 20:12:43eric.snowsetnosy: + eric.snow
messages: + msg147254
2011-11-07 10:16:21python-devsetstatus: open -> closed
resolution: fixed
messages: + msg147216

stage: patch review -> resolved
2011-11-07 09:12:07vinay.sajipsetmessages: + msg147214
2011-11-07 09:00:32floxsetmessages: + msg147213
2011-11-07 08:56:38floxsetfiles: + issue13361_dont_check.diff

messages: + msg147212
stage: needs patch -> patch review
2011-11-07 08:55:14python-devsetnosy: + python-dev
messages: + msg147211
2011-11-07 08:53:37floxsetfiles: + issue13361_check.diff
keywords: + patch
2011-11-06 23:47:02floxsetnosy: + vinay.sajip
2011-11-06 23:46:24floxcreate