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

Prevent None extension parameters from crashing putup #510

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

abravalheri
Copy link
Collaborator

Purpose

As shown in #506, ConfigUpdater will crash when trying to persist option = None.
This is problematic when an extension defines a "persitible" parameter that can assume None, since it may crash putup.

These changes implement a workaround for that.

Approach

Force the parameter to be persisted as an empty string instead of None?
This might be controversial, but it is the safest approach for an INI format, since INI files are opaque in terms of types for option values (in the end of the day they are always strings and it is up for the users to interpret those strings the way they see fit).

Alternative (not implemented) and Cons:

We could use allow_no_value=True, however other tools using the setup.cfg (e.g. setuptools, isort, pytest, mypy, etc...), might not like this (the default value for allow_no_value in ConfigParser is False), and therefore parser errors might crash them.

As shown in #506, ConfigUpdater will crash when trying to persist
`option = None`.

These changes implement a workaround for that.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 5350649215844352

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 97.29%

Totals Coverage Status
Change from base Build 5113446040600576: 0.005%
Covered Lines: 1635
Relevant Lines: 1667

💛 - Coveralls

@FlorianWilhelm
Copy link
Member

Thanks @abravalheri!

@FlorianWilhelm FlorianWilhelm merged commit bf08655 into master Oct 1, 2021
@FlorianWilhelm FlorianWilhelm deleted the issue-506-none-in-setupcfg branch October 1, 2021 12:28
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

Successfully merging this pull request may close these issues.

None yet

3 participants