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

Handle errors when default section(s) forgotten #1572

Merged
merged 3 commits into from Oct 27, 2020

Conversation

jaydesl
Copy link
Contributor

@jaydesl jaydesl commented Oct 21, 2020

Trying this one from a different angle...

Previously, if any of the default sections were omitted in a setup.cfg that defined a sections block, isort would throw an exception (see linked issues). Any omitted default sections are now appended to chain(sections, forced_separate) during parsing. Duplicates are avoided and the correct (default) order is maintained with an OrderedDict().

Fixes #1570 #1562

@timothycrosley
Copy link
Member

Thanks for working on this! I'm wondering if it isn't better to hard fail in these cases, but just improve the documentation and presentation of the error message?

@anirudnits
Copy link
Collaborator

anirudnits commented Oct 22, 2020

@timothycrosley I agree, it seems more of a documentation issue as discussed here #1562.

@jaydesl jaydesl force-pushed the develop branch 2 times, most recently from 3683f54 to ddd6196 Compare October 22, 2020 12:59
@jaydesl jaydesl changed the title Fix KeyError when default section(s) forgotten Handle errors when default section(s) forgotten Oct 22, 2020
@jaydesl
Copy link
Contributor Author

jaydesl commented Oct 22, 2020

Here's another take:

  • docs updated
  • warn when default sections omitted in config file
  • warn on each KeyError during parsing

I took to using warnings because I couldn't find a good example of a hard fail anywhere else in isort, but those warnings create a lot of noise.

Maybe a hard-fail similar to the catch-all is better?
eg

    except KeyError as error:
        if error.args[0] in DEFAULT_CONFIG.sections:
            printer = create_terminal_printer(color=config.color_output)
            printer.error(
                f"Parsing failed because the default {SECTION} is missing in `sections` config")\n"
                "Please read more here: https://pycqa.github.io/isort/#custom-sections-and-ordering"
            )
            raise

@jaydesl
Copy link
Contributor Author

jaydesl commented Oct 25, 2020

i.e something like this

@timothycrosley
Copy link
Member

This looks pretty good to me! Thanks @jaydesl!

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.

ERROR: Unrecoverable exception thrown ... This should NEVER happen.
3 participants