classification
Title: configparser leaks implementation detail
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lukasz.langa Nosy List: Claudiu.Popa, lukasz.langa, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2013-11-10 17:16 by Claudiu.Popa, last changed 2014-09-04 08:45 by Claudiu.Popa. This issue is now closed.

Files
File name Uploaded Description Edit
configparser.patch Claudiu.Popa, 2013-11-10 17:16 review
issue19546.patch Claudiu.Popa, 2014-04-14 18:29 Remove trailing whitespace. review
issue19546_1.patch Claudiu.Popa, 2014-06-27 06:01 Small style fixes. review
Messages (14)
msg202538 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-10 17:16
Various exceptions raised by configparser module leaks implementation detail, by chaining KeyErrors, as seen below:

Python 3.4.0a4+ (default:0aa2aedc6a21+, Nov  5 2013, 17:10:42)
[GCC 4.2.1 20070831 patched [FreeBSD]] on freebsd8
Type "help", "copyright", "credits" or "license" for more information.
>>> import configparser
>>> parser = configparser.ConfigParser()
>>> parser.remove_option('Section1', 'an_int')
Traceback (most recent call last):
  File "/tank/libs/cpython/Lib/configparser.py", line 935, in remove_option
    sectdict = self._sections[section]
KeyError: 'Section1'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tank/libs/cpython/Lib/configparser.py", line 937, in remove_option
    raise NoSectionError(section)
configparser.NoSectionError: No section: 'Section1'
>>>

There are multiple places where this happens: using basic and extended interpolation, using .options to retrieve non-existent options, using .get or .remove_options for non-existent options/sections. The attached patch tries to fixes all those issues by suppressing the initial exception.
msg202552 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-11-10 19:04
I'd vote -1 on this one.  The extra context in this case is not confusing, and might be helpful to someone.
msg215334 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-01 17:33
But the last traceback conveys enough information, the user can see immediately that the given section does not exist.
My problem with the current behaviour is that the first error distracts the user, while the actual problem is the second traceback.
But I have another example which might contradict my proposal. In the example in the second traceback, it's not clear what `Bad value substitution` stands for and the first traceback adds the relevant context (the fact that key is missing). But I guess this stands for another issue, for enhancing the message for InterpolationMissingOptionError to transmit the fact that a given option is missing.


Traceback (most recent call last):
  File "C:\Python34\lib\configparser.py", line 410, in _interpolate_some
    v = map[var]
  File "C:\Python34\lib\collections\__init__.py", line 789, in __getitem__
    return self.__missing__(key)            # support subclasses that define __missing__
  File "C:\Python34\lib\collections\__init__.py", line 781, in __missing__
    raise KeyError(key)
KeyError: 'home_dir1'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python34\lib\configparser.py", line 1204, in __getitem__
    return self._parser.get(self._name, key)
  File "C:\Python34\lib\configparser.py", line 773, in get
    d)
  File "C:\Python34\lib\configparser.py", line 374, in before_get
    self._interpolate_some(parser, option, L, value, section, defaults, 1)
  File "C:\Python34\lib\configparser.py", line 413, in _interpolate_some
    option, section, rest, var)
configparser.InterpolationMissingOptionError: Bad value substitution:
        section: [Paths]
        option : my_dir
        key    : home_dir1
        rawval : /lumberjack
msg215336 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-04-01 18:15
Why?  Just let the context convey the information.  It's not like we are building a UI here, this is for the programmer.
msg215337 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-04-01 18:19
Although I will grant you that I have to guess at what the bad value substitution error message is trying to tell me, so that error message could use some improvement.
msg215590 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-05 10:36
I've created a new issue for the InterpolationMissingOptionError message, issue21159. This issue can be closed.
msg215591 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-05 10:36
Ups, sorry for the change of resolution.
msg215604 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-04-05 13:19
I've been thinking about this more, and I think I will revise my opinion.  I haven't been able to think of a place where knowing that the key is missing in self._sections would in fact be helpful to a programmer using the module.  So I'm OK with this being an example of a place where the new error fully encapsulates the information from the context and therefore the context can be suppressed.
msg215621 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2014-04-05 16:52
FWIW I agree with Claudiu that the internal exceptions are an implementation detail. If we ever made, say, a SQLite-based or memcache-based configparser, those would be different, but the external API would stay the same.

Will fix.
msg216159 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-14 18:29
If there is anything left to do for this patch, please tell me.
msg221674 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-06-27 06:01
Łukasz, do you have some time to take a look at this patch?
msg226350 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-04 08:42
New changeset 2b14665b7bce by Łukasz Langa in branch 'default':
Fix #19546: onfigparser exceptions expose implementation details.  Patch by Claudiu Popa.
http://hg.python.org/cpython/rev/2b14665b7bce

New changeset 554ead559f24 by Łukasz Langa in branch 'default':
Merge fix for #19546: configparser exceptions leak implementation details
http://hg.python.org/cpython/rev/554ead559f24
msg226351 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2014-09-04 08:44
Thank you Claudiu for your patch, sorry it took so long to respond.
msg226352 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-09-04 08:45
No problem, thank you for committing it.
History
Date User Action Args
2014-09-04 08:45:13Claudiu.Popasetmessages: + msg226352
2014-09-04 08:44:18lukasz.langasetmessages: + msg226351
2014-09-04 08:43:53lukasz.langasetstatus: open -> closed
resolution: fixed
stage: resolved
2014-09-04 08:42:48python-devsetnosy: + python-dev
messages: + msg226350
2014-06-27 06:01:27Claudiu.Popasetfiles: + issue19546_1.patch
type: behavior
messages: + msg221674

versions: + Python 3.5
2014-04-14 18:29:29Claudiu.Popasetfiles: + issue19546.patch

messages: + msg216159
2014-04-05 16:52:57lukasz.langasetmessages: + msg215621
2014-04-05 13:19:58r.david.murraysetmessages: + msg215604
2014-04-05 10:36:51Claudiu.Popasetresolution: third party -> (no value)
messages: + msg215591
2014-04-05 10:36:22Claudiu.Popasetresolution: third party
messages: + msg215590
2014-04-01 18:19:01r.david.murraysetmessages: + msg215337
2014-04-01 18:15:52r.david.murraysetmessages: + msg215336
2014-04-01 17:33:01Claudiu.Popasetmessages: + msg215334
2013-11-10 21:37:31lukasz.langasetassignee: lukasz.langa
2013-11-10 19:04:21r.david.murraysetnosy: + r.david.murray
messages: + msg202552
2013-11-10 17:16:52Claudiu.Popacreate