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

configparser does not convert defaults to strings #68023

Closed
aragilar mannequin opened this issue Apr 1, 2015 · 16 comments
Closed

configparser does not convert defaults to strings #68023

aragilar mannequin opened this issue Apr 1, 2015 · 16 comments
Labels
3.7 deferred-blocker stdlib type-bug

Comments

@aragilar
Copy link
Mannequin

@aragilar aragilar mannequin commented Apr 1, 2015

BPO 23835
Nosy @warsaw, @vstinner, @bitdancer, @ambv, @aragilar, @asottile, @miss-islington, @JunyiXie
PRs
  • #2558
  • #3176
  • #3191
  • #7524
  • #7529
  • Files
  • py27-config-fix.patch
  • 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 2018-06-08.17:10:28.583>
    created_at = <Date 2015-04-01.04:23:08.378>
    labels = ['3.7', 'deferred-blocker', 'type-bug', 'library']
    title = 'configparser does not convert defaults to strings'
    updated_at = <Date 2021-03-13.13:23:31.799>
    user = 'https://github.com/aragilar'

    bugs.python.org fields:

    activity = <Date 2021-03-13.13:23:31.799>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-06-08.17:10:28.583>
    closer = 'barry'
    components = ['Library (Lib)']
    creation = <Date 2015-04-01.04:23:08.378>
    creator = 'aragilar'
    dependencies = []
    files = ['38861']
    hgrepos = []
    issue_num = 23835
    keywords = ['patch', '3.7regression']
    message_count = 16.0
    messages = ['239768', '240249', '297793', '298205', '298234', '298279', '300660', '300661', '300717', '300791', '318983', '318999', '319047', '319066', '320899', '323285']
    nosy_count = 8.0
    nosy_names = ['barry', 'vstinner', 'r.david.murray', 'lukasz.langa', 'aragilar', 'Anthony Sottile', 'miss-islington', 'JunyiXie']
    pr_nums = ['2558', '3176', '3191', '7524', '7529']
    priority = 'deferred blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue23835'
    versions = ['Python 3.7']

    @aragilar
    Copy link
    Mannequin Author

    @aragilar aragilar mannequin commented Apr 1, 2015

    ConfigParser(defaults={1:2.4}) and ConfigParser(defaults={"a":5.2}) cause an exception when configparser tries to perform string operations on 1 and 5.2. I didn't see it documented that defaults must only contain strings, and using ConfigParser['DEFAULT'] = {1:2.4} does the necessary string conversion, so it looks like there should be some conversion done to the defaults.

    @aragilar aragilar mannequin added type-crash stdlib labels Apr 1, 2015
    @serhiy-storchaka serhiy-storchaka added type-bug and removed type-crash labels Apr 1, 2015
    @aragilar
    Copy link
    Mannequin Author

    @aragilar aragilar mannequin commented Apr 8, 2015

    Here's a patch for 2.7 (based of the head of the 2.7 branch), something similar could be done for 3.4 (I wasn't sure what branch I was supposed to base the patch off, since 3.4 is inactive). The string requirement was already noted in the docstring of the configparser module, but that's not mentioned in the main docs. Also, I wasn't sure where to put a test in because there was not test_configparser.py located in Lib/test.

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Jul 6, 2017

    I'm guessing we can only do something here in 3.7, for backward compatibility reasons, but maybe I'm wrong. Hopefully Lukasz will notice the activity on the issue and have time to take a look.

    @bitdancer bitdancer added the 3.7 label Jul 6, 2017
    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jul 12, 2017

    Responded on the PR.

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Jul 12, 2017

    Thanks. OK, so you agree a fix is appropriate. What about the question of backport/backward compatibility?

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jul 13, 2017

    Backwards compatibility will be considered when the patch is ready. I'm not overly worried about fixing a case that currently raises exceptions all over the place. There's few compatibility concerns that we need to consider in this case.

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Aug 21, 2017

    New changeset 44e6ad8 by Łukasz Langa (James Tocknell) in branch 'master':
    bpo-23835: Enforce that configparser defaults are strings (bpo-2558)
    44e6ad8

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Aug 21, 2017

    New changeset ea57923 by Łukasz Langa in branch 'master':
    bpo-23835: [docs] configparser converts defaults to strings (bpo-3176)
    ea57923

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Aug 22, 2017

    I merged the original fix and documented it. I thought about it some more and remembered that RawConfigParser objects do in fact support non-string values by historical coincidence. It's unfortunately a popular idiom with old programs to load some configuration defaults using the defaults= keyword and later use the legacy get() and set() API which doesn't check types inside. A config file like this cannot be safely written back to a file, etc.

    I would very much like to get rid of RawConfigParser entirely but we're stuck with it due to backwards compatibility. So, to fix the regression, I created PR 3191.

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Aug 24, 2017

    New changeset a5fab17 by Łukasz Langa in branch 'master':
    bpo-23835: Restore legacy defaults= behavior for RawConfigParser (bpo-3191)
    a5fab17

    @ambv ambv closed this as completed Aug 27, 2017
    @warsaw
    Copy link
    Member

    @warsaw warsaw commented Jun 7, 2018

    I think this introduced a regression in 3.7. See bpo-33802

    https://bugs.python.org/issue33802

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Jun 8, 2018

    It seems like this issue introduced a regression according to Barry: bpo-33802.

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jun 8, 2018

    New changeset 214f18e by Łukasz Langa in branch 'master':
    bpo-33802: Do not interpolate in ConfigParser while reading defaults (GH-7524)
    214f18e

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jun 8, 2018

    New changeset f44203d by Miss Islington (bot) in branch '3.7':
    bpo-33802: Do not interpolate in ConfigParser while reading defaults (GH-7524)
    f44203d

    @warsaw warsaw closed this as completed Jun 8, 2018
    @asottile
    Copy link
    Mannequin

    @asottile asottile mannequin commented Jul 2, 2018

    Unclear if this regression (from this patch) is intentional or not:

    $ python3.6 -c 'import configparser; configparser.ConfigParser(defaults={"a": None})'
    $ python3.7 -c 'import configparser; configparser.ConfigParser(defaults={"a": None})'
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/configparser.py", line 638, in __init__
        self._read_defaults(defaults)
      File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/configparser.py", line 1216, in _read_defaults
        self.read_dict({self.default_section: defaults})
      File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/configparser.py", line 753, in read_dict
        self.set(section, key, value)
      File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/configparser.py", line 1197, in set
        self._validate_value_types(option=option, value=value)
      File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/configparser.py", line 1182, in _validate_value_types
        raise TypeError("option values must be strings")
    TypeError: option values must be strings
    

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Aug 8, 2018

    That's intentional. In ConfigParser objects this exception would be raised for any other section's assignment already. You want RawConfigParser if you want to put (invalid) types as option values. See:

    >>> cp = ConfigParser()
    >>> cp['asd'] = {'a': None}
    Traceback (most recent call last):
    ...
    TypeError: option values must be strings
    >>> rcp = RawConfigParser()
    >>> rcp['asd'] = {'a': None}
    >>>

    @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 deferred-blocker stdlib type-bug
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants