[develop] Fix issue 541: correct error message when config.yaml has invalid entries#559
Merged
MichaelLueken merged 2 commits intoJan 31, 2023
Merged
Conversation
1. Fix error message that appears when user specifies an invalid key in their config.yaml file. 2. In order to achieve the above neatly, I had to change the behavior of check_structure_dict in python_utils. Rather than simply printing the invalid key/value pair, it now returns a dictionary of invalid key/value pairs (that is empty if all keys are valid). I also fixed a minor bug here: even though this function *claimed* to detect all invalid entries, it actually only printed the first before returning. Now all invalid entries are returned.
Collaborator
Author
|
Note: The unit test failure appears to be a known issue introduced in #536 |
MichaelLueken
approved these changes
Jan 26, 2023
Collaborator
MichaelLueken
left a comment
There was a problem hiding this comment.
@mkavulich These changes look good to me! Approving now and then will launch the Jenkins tests.
zmoon
reviewed
Jan 26, 2023
Suggestions for correct docstring style from @zmoon Co-authored-by: Zachary Moon <zmoon92@gmail.com>
christinaholtNOAA
approved these changes
Jan 31, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DESCRIPTION OF CHANGES:
This is actually two fixes/enhancements, one required by the other:
check_structure_dict(). Rather than simply printing the invalid key/value pair, it now returns a dictionary of invalid key/value pairs (that is empty if all keys are valid). I also fixed a minor bug here: even though this function claimed to detect all invalid entries, it actually only printed the first before returning. Now all invalid entries are returned.With this change users will now see the following type of error if they specify one or more invalid keys in their config.yaml file:
Type of change
TESTS CONDUCTED:
Ran unit tests for python_utils which were successful. Ran several iterations of incorrect config.yaml entries on Hera and Cheyenne, which produced expected results. Also ran fundamental suite of tests on Hera as a sanity check, but this change should not impact correctly configured runs.
DEPENDENCIES:
None
DOCUMENTATION:
None
ISSUE:
Fixes #541
CHECKLIST
CONTRIBUTORS:
Thank you to @zmoon for taking the time to open an issue describing this problem.