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

config parsing inconsistency between Python 2 and Python 3 #2917

Closed
thomasgilgenast opened this issue Mar 9, 2020 · 2 comments
Closed

config parsing inconsistency between Python 2 and Python 3 #2917

thomasgilgenast opened this issue Mar 9, 2020 · 2 comments

Comments

@thomasgilgenast
Copy link

I'm using luigi in a library that supports both Python 2 and Python 3 from the same code base, and I'm trying to ensure that the same luigi config file can be used with both Python 2 and Python 3. I'm encountering an inconsistency between how the Python 2 ConfigParser and Python 3 configparser handle escaping of "%" symbols in the "ini/cfg"-style config file. configparser requires that "%" symbols be escaped (by writing "%%"; the extra "%" gets removed when accessing values from the config file), while ConfigParser does not ("%" symbols in the config file are fine).

A minimal example to reproduce this is (run in Python 2 with configparser installed):

>>> import ConfigParser
>>> config2 = ConfigParser.ConfigParser()
>>> config2.add_section('section')
>>> config2.set('section', 'unescaped', '10%')
>>> config2.set('section', 'escaped', '10%%')
>>> config2.get('section', 'escaped')  # bad because extra % in parsed value
'10%%'
>>>
>>> import configparser
>>> config3 = configparser.ConfigParser()
>>> config3.add_section('section')
>>> config3.set('section', 'unescaped', '10%')  # bad because ValueError
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "...\backports\configparser\__init__.py", line 1305, in set
    super(ConfigParser, self).set(section, option, value)
  File "...\backports\configparser\__init__.py", line 978, in set
    value = self._interpolation.before_set(self, section, option, value)
  File "...\backports\configparser\__init__.py", line 439, in before_set
    "position %d" % (value, tmp_value.find('%'))
ValueError: invalid interpolation syntax in u'10%' at position 2
>>> config3.set('section', 'escaped', '10%%')
>>> config3.get('section', 'escaped')
u'10%'

Not escaping the % symbol is not an option, because it will lead to a ValueError in Python 3. Escaping the % symbol is also not an option, because it will lead to different results between Python 2 and Python 3 ("10%%" vs "10%").

A related error that I've seen when parsing "unescaped" config files in Python 3 is:

configparser.InterpolationSyntaxError: '%' must be followed by '%' or '('

As far as I can tell, this is not related to #2527 / #2354 since this difference is not "caused" by luigi - it is caused by differences in the underlying ConfigParser/configparser implementations.

The Python-Future cheat sheet suggests that one way to make code that uses ConfigParser/configparser behave the same way in both Python 2 and Python 3 is to install configparser (available as a backport) when running the code in Python 2. Unfortunately, luigi currently attempts to import ConfigParser first, so even if configparser is available in a Python 2 environment, luigi will still use ConfigParser.

This suggests that a possible solution to this is to both

  1. require configparser in my client library for versions of Python that don't have it via install_requires=['configparser;python_version<"3.2"'] in my library's setup.py
  2. re-order the attempted order of imports in luigi so that configparser is tried first

I am not sure if there are any disadvantages to attempting to import configparser first.

This might be made irrelevant by plans to drop Python 2 support anyway (#2830) - if so feel free to close this.

@thomasgilgenast
Copy link
Author

I've created an implementation of (2) in #2918 to confirm that changing the order of the import attempts would fix my issue, but I'm definitely not 100% sure yet that this is the right/only way to address this - I'm open to other solutions/suggestions/discussion.

@Tarrasch
Copy link
Contributor

Python 2

Python 2 is not supported in luigi anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants