No traceback on missing configuration with towncrier check

[chrysle]

Description

Fixes #336

Checklist

• Make sure changes are covered by existing or new tests.
• For at least one Python version, make sure local test run is green.
• Create a file in src/towncrier/newsfragments/. Describe your
  change and include important information. Your change will be included in the public release notes.
• Make sure all GitHub Actions checks are green (they are automatically checking all of the above).
• Ensure docs/tutorial.rst is still up-to-date.
• If you add new CLI arguments (or change the meaning of existing ones), make sure docs/cli.rst reflects those changes.
• If you add new configuration options (or change the meaning of existing ones), make sure docs/configuration.rst reflects those changes.

[adiroiban]

Thanks for the changes.

Changes looks good, and I guess that they work for this specific use case.

I am not a big fan us using sys.exit

At the same time, I see that ConfigError is raised in many other places.
Does towncrier already handle those cases without a traceback?

For example, if you have a config with single_file = Non-Boolean-Value , do you see a clean error? or also the traceback.

I wish we could just do a generic handling of the ConfigError instead of replacing it with sys.exit case by case.

I am leaving this a bit longer for review to see that others are saying.

If nobody else is against this, I guess that we can merge it, and look at creating a general fix in a separate PR.

This PR has a new test and that is already an improvement :)

[SmileyChris]

I am not a big fan us using sys.exit

At the same time, I see that ConfigError is raised in many other places.
Does towncrier already handle those cases without a traceback?

The _main function (wrapped as the build command) in build.py wraps the call in a try/except ConfigError and sys.exits already so the precident is set. This PR should just follow the same pattern.

[SmileyChris]

I wish we could just do a generic handling of the ConfigError instead of replacing it with sys.exit case by case.

Well actually, these could be changed to a click.ClickException and then it'd be handled automagically...

I added this to check.pys _main as an example:

    try:
        __main(compare_with, directory, config)
    except ConfigError as e:
        raise click.ClickException(str(e))

Which results in towncrier check in a no-configuration file location outputting (with an exit code of 1):

Error: No configuration file found.
Looked in: /home/chris/sandbox/abc

[adiroiban]

Thanks Chris for the feedback.
I think that raise click.ClickException(str(e)) is the way to go.

No need to reinvent the wheel :)

[adiroiban]

Thanks @chrysle for the initial PR

Can you please consider using the dedicated ClickException ?

I guess that one option is to go with something like this

-class ConfigError(Exception):
+class ConfigError(click.ClickException):

I don't know if we need multiple exception types in towncrier
and I can't see any side-effect for this change.

[chrysle]

Thanks for the review! I updated the code.

However, there seems to be an issue with urllib3 after dropping an OpenSSL version which breaks the doctests. See urllib3/urllib3#2168

[adiroiban]

I think this looks good.
Only minor comments.
Many thanks for the test.

---

I hope the CI will be green and we can merge this.

Thanks again

[chrysle]

Updated! Thanks for reviewing.

[hynek]

Posted from a phone on a car ;)

[chrysle]

@hynek Thanks for review! Updated.

[chrysle]

I knew the OG mktemp is deprecated because of race condition problems but I was under the wrong impression that trial did it right.

@hynek So let's deprecate that thing, add a modern solution and do a great clean-out at towncrier!

Where can I volunteer?

[hynek]

That code lives in Twisted proper.