Repository navigation
Emphasising in the docs configparser.SafeConfigParser over ConfigParser #50766
Description
| BPO | 6517 |
|---|---|
| Nosy | @freddrake, @birkenfeld, @rhettinger, @merwok, @bitdancer, @voidspace, @ambv |
| Superseder | |
| Files |
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/ambv'
closed_at = <Date 2010-11-22.03:18:56.272>
created_at = <Date 2009-07-18.20:36:38.767>
labels = ['type-bug', 'docs']
title = 'Emphasising in the docs configparser.SafeConfigParser over ConfigParser'
updated_at = <Date 2010-11-22.03:18:56.271>
user = 'https://bugs.python.org/till'bugs.python.org fields:
activity = <Date 2010-11-22.03:18:56.271>
actor = 'lukasz.langa'
assignee = 'lukasz.langa'
closed = True
closed_date = <Date 2010-11-22.03:18:56.272>
closer = 'lukasz.langa'
components = ['Documentation']
creation = <Date 2009-07-18.20:36:38.767>
creator = 'till'
dependencies = []
files = ['14522', '14523']
hgrepos = []
issue_num = 6517
keywords = ['patch']
message_count = 33.0
messages = ['90689', '90693', '90694', '90695', '90696', '111411', '111424', '111430', '111472', '111617', '111620', '111628', '111637', '112571', '112573', '112574', '112575', '112580', '112581', '112586', '112589', '112590', '112593', '112598', '112599', '112601', '112602', '112606', '112618', '112619', '112638', '112643', '122079']
nosy_count = 8.0
nosy_names = ['fdrake', 'georg.brandl', 'rhettinger', 'eric.araujo', 'r.david.murray', 'michael.foord', 'till', 'lukasz.langa']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = 'patch review'
status = 'closed'
superseder = '10499'
type = 'behavior'
url = 'https://bugs.python.org/issue6517'
versions = ['Python 3.2']
Activity
till commentedon Jul 18, 2009
There seems to be no way to add a config item with a value containing a
formatstring without this formatstring beeing handled by configparser.
A possible way to escape the formatstrings could be to double the
%-sign, e.g.:
[foo]
bar = %%(string)s
The value of the bar item should then be "%(string)s".
bitdancer commentedon Jul 18, 2009
The formatstring is a Python % formatting code, and doubling the %% is
the way to escape one, so what you suggest is in fact the way it works.
Do you have a test case demonstrating that it doesn't work as
documented (it works for me)?
till commentedon Jul 18, 2009
Afacs it is not documented to work. Here is the testcase:
#!/usr/bin/python
# vim: fileencoding=utf8
print config.get("s", "f")
bitdancer commentedon Jul 18, 2009
Ah, that's because you aren't using SafeConfigParser (which the docs
recommend that you do unless you have to use ConfigParser for backward
compatibility reasons). Your test case works with SafeConfigParser.
You are correct that it is not explicitly documented in the ConfigParser
docs. It is implied by the fact that the magic substitution is
documented to be a standard format string, and by the recommendation to
use SafeConfigParser because ConfigParser's handling of formatstrings is
broken.
To make this all explicit, I've attached a doc patch to make the
motivation for using SafeConfigParser clearer, using this as the
example. Let me know what you think.
Georg, Raymond, I added you as nosy to ask for a style/content opinion.
Should we instead rewrite the section to put SafeConfigParser first,
and perhaps even deprecate ConfigParser?
till commentedon Jul 18, 2009
It would be nice if you could expand the patch to also use only one name
for the support of format strings.
In the beginning it is introduced as "reference expansion", but later in
the document, several variatons of interpolation are used: "magical
interpolation", "string interpolation", "%" interpolation is used.
Therefore I guess it should also be introduced as interpolation.
Another nice improvement would also be to move the ConfigParser to a
"deprecated classes" section at the end like it is done for some methods
in the string module.
ambv commentedon Jul 24, 2010
This issue superseeds bpo-8888 because of the patches attached.
Brett, there are two ways we can solve this issue:
I personally would go with 1. and maybe just "hint" about the fact that ConfigParser is not for end users. Formal deprecation seems too big for this since the regular ConfigParser still does have its valid use cases.
Please decide on what we're going to do and I'll edit the docs and add the tests here to py3k.
freddrake commentedon Jul 24, 2010
I disagree.
The documentation should promote RawConfigParser, and note
SafeConfigParser and ConfigParser as remaining for backward
compatibility for existing software. Maintainers of legacy software
using ConfigParser should be encouraged to convert to SafeConfigParser
(or even RawConfigParser) if possible.
Documentation changes should be sufficient; deprecation warnings
typically generate more pain than good.
ambv commentedon Jul 24, 2010
That is another way to go around this. Anyway, ConfigParser is the least predictable of all three for end users and the documentation should be adjusted to emphasize this.
That's another comment that should appear in the documentation.
Isn't is what I was saying above?
freddrake commentedon Jul 24, 2010
We're in agreement; I was specifically adding weight to *not*
selecting the second optoin Łukasz Langa presented:
voidspace commentedon Jul 26, 2010
This should just be applied. I'll do it shortly unless there is an objection.
13 remaining items
freddrake commentedon Aug 3, 2010
2010/8/3 Łukasz Langa <[email protected]>:
I'd rather see the class renamed and SafeConfigParser made the alias in 3.2.
Otherwise, +1 for this plan (msg 112589), as there's no silent breakage.
voidspace commentedon Aug 3, 2010
I'd be happy with aliasing SafeConfigParser to ConfigParser in 3.2. Can we just do this without a deprecation process?
freddrake commentedon Aug 3, 2010
Making ConfigParser an alias for SafeConfigParser creates a silent
behavioral change. An application developer may not realize that
users rely on the "full" ConfigParser anti-glory and end up breaking
their configurations without so much as providing a heads-up in the
application news.
-1
So long as the application developer has to change their code to move
away from ConfigParser, there's a better chance of affected end users
receiving a heads-up.
ambv commentedon Aug 3, 2010
Unfortunately, I have to agree with Fred here. We'll stick to renaming and the deprecation process.
voidspace commentedon Aug 3, 2010
Sorry - I misunderstood your earlier suggestion Fred.
configparser.ConfigParser is the *natural* name for SafeConfigParser. I'm strongly +1 on moving towards that. (I doubt there would *actually* be any real code breakage if we did it earlier though ;-)
voidspace commentedon Aug 3, 2010
By the way, given that deprecation warnings are silent I am strongly -1 on removing the ConfigParser name altogether. That would cause far more breakage. As ConfigParser should not be used at all, and SafeConfigParser provides its functionality minus bugs, SafeConfigParser (horrible name) should replace it.
merwok commentedon Aug 3, 2010
Agree on the proposal of Łukasz, with the caveat mentioned by Fred (rename the class and make the old name an alias, for pickle and all). I’ll let Michael and Fred decide if the name ConfigParser has to go or not, I’m happy enough that the class will be deprecated and removed.
voidspace commentedon Aug 3, 2010
Getting *rid* of the name ConfigParser would be annoying and cause *gratuitous* code breakage.
If we are going to keep the name but get rid of the "unsafe" version then we can only replace it with what is now SafeConfigParser - as it is almost entirely compatible with it. (Modulo the bug fixing "behavioural change".)
So the real choices are:
Only the last of these is a positive step forwards... :-)
(Strongly -1 on introducing *yet another name* to refer to these classes by.)
ambv commentedon Aug 3, 2010
There IS one more option that seems to be better than all of the above:
We can do all this for 3.2 I guess without inflicting any actual damage. It will fix more bugs in running code that break config files.
Maybe we shouldn't be so afraid after all and just clean it up.
freddrake commentedon Aug 3, 2010
It doesn't make sense to make any of these changes to Python 2; this
really should have been separate from the documentation issue. That's
probably understood by everyone, but explicit is better.
Merging implementations
-----------------------
I've no objection to merging RawConfigParser and SafeConfigParser,
using a constructor argument to control whether interpolation is
performed. It's not clear this provides any improvement in
maintainability or usage.
Isolating the old (broken interpolation) ConfigParser behavior so it's
not in the middle of the inheritance stack would be good.
Changing ConfigParser behavior
------------------------------
Changing the behavior of the ConfigParser name requires the deprecation
process. We may think nobody in their right mind is using that, but
changing something out from under app developers is really bad. This
should be considered a problem for configparser as outlined in msg 112598.
Whether we can assign new semantics to the ConfigParser name in the
future is questionable. I think Python's compatibility policy allows
it, but that doesn't make it a good idea.
(This seems to be the real sticking point, unfortunately.)
Class names
-----------
I don't understand Michael's objection to adding new names for the
configparser classes; that's one of the few points where we don't run
into backward-compatibility black holes.
In the case of a merged implementation, a new name for the merged class
(possibly "Configuration") may be the best bet; the old names can then
be subclasses of that which generate appropriate deprecation warnings.
voidspace commentedon Aug 3, 2010
If we merge the functionality in a single class with a new name then I guess that is fine as it will simplify the documentation rather than complexify it (good word hey). We still need to *mention* the old names so that people finding them in old code can find an up to date reference on them.
Here's what I don't understand about Fred's difficulty with replacing ConfigParser with the sane implementation.
After we deprecate ConfigParser as it is now we have two choices.
I don't see how the first option could *in any way* be preferable to the second.
ambv commentedon Nov 22, 2010
Hopefully both RawConfigParser and ConfigParser will be successfully deprecated as part of bpo-10499. The discussion goes on there.