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

Allow exception to bubble up when priority file is bad. #487

Closed

Conversation

elebow
Copy link
Contributor

@elebow elebow commented Mar 27, 2020

Description of proposed changes

In filter, --priority is an optional argument. When a user provides that argument, they are expressing an intent for the program to use those priorities in its calculations.

If the specified file is missing or malformed, then the user's expressed intent cannot be carried out. The program should abort noisily, so the user can correct the problem or adjust their expressed intent.

The legacy behavior did print an "ERROR:" message, but that's easy for the user to miss. It's more reliable to let the exception bubble up and abort execution.

Related issue(s)

none

Testing

Unit tests included

@elebow elebow force-pushed the allow-exceptions-from-bad-priority-file branch from ded2d60 to d44cc86 Compare March 27, 2020 04:36
@tacaswell
Copy link
Contributor

Might be worth still printing out the "ERROR: ..." next in case someone is screen-scraping the output?

When the user specifies a priority file, the program should either obey
the user's instructions or fail. A "best effort" approach with default
values may result in the program doing something that the user did not
intend.

With "fail on error" behavior, user can have confidence that if the
program completes, it followed the instructions as specified.
@elebow elebow force-pushed the allow-exceptions-from-bad-priority-file branch from d44cc86 to f10e20a Compare March 28, 2020 02:04
@elebow
Copy link
Contributor Author

elebow commented Mar 28, 2020

@tacaswell good idea. I re-added "ERROR:" output.

@huddlej
Copy link
Contributor

huddlej commented Mar 30, 2020

I messed up the git history by trying to resolve conflicts in test_filter.py, so I've pushed those resolved changes in commit 520b1ff and will close this now.

Thank you, @elebow!

@huddlej huddlej closed this Mar 30, 2020
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.

3 participants