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
configparser leaks implementation detail #63745
Comments
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. |
I'd vote -1 on this one. The extra context in this case is not confusing, and might be helpful to someone. |
But the last traceback conveys enough information, the user can see immediately that the given section does not exist. 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 |
Why? Just let the context convey the information. It's not like we are building a UI here, this is for the programmer. |
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. |
I've created a new issue for the InterpolationMissingOptionError message, bpo-21159. This issue can be closed. |
Ups, sorry for the change of resolution. |
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. |
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. |
If there is anything left to do for this patch, please tell me. |
Łukasz, do you have some time to take a look at this patch? |
New changeset 2b14665b7bce by Łukasz Langa in branch 'default': New changeset 554ead559f24 by Łukasz Langa in branch 'default': |
Thank you Claudiu for your patch, sorry it took so long to respond. |
No problem, thank you for committing it. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: