-
Notifications
You must be signed in to change notification settings - Fork 64
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
Return full error message from xmllint #586
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks for fixing this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output looks incorrect if a command has output to both stdout
and stderr
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@peverwhee These changes look good to me. Thanks!
Tag name (required for release branches): Originator(s): peverwhee Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number): 1. Adds call to new CCPP register phase brought in with framework PR [#582](NCAR/ccpp-framework#582) 2. Uses the full error message returned from the exception handled when xmllint is called (updated in framework PR [#586](NCAR/ccpp-framework#586)) 3. Adds `inverse_exner_function_wrt_surface_pressure` as possible input variable name for exner for backwards compatibility with old converted snapshots closes #215 (add optional register phase) closes #286 (Improve error message returned by XML linter) Describe any changes made to build system: None Describe any changes made to the namelist: None List any changes to the defaults for the input datasets (e.g. boundary datasets): None List all files eliminated and why: None List all files added and what they do: None List all existing files that have been modified, and describe the changes: (Helpful git command: `git diff --name-status development...<your_branch_name>`) M .gitmodules - brings in new CCPP framework tag M src/control/cam_comp.F90 M src/physics/utils/phys_comp.F90 - Adds call to new CCPP register phase in the generated cap M src/data/generate_registry_data.py - Uses full error message returned from xmllint M src/data/registry.xml - add backwards-compatible exner name If there are new failures (compared to the `test/existing-test-failures.txt` file), have them OK'd by the gatekeeper, note them here, and add them to the file. If there are baseline differences, include the test and the reason for the diff. What is the nature of the change? Roundoff? derecho/intel/aux_sima: all pass derecho/gnu/aux_sima: all pass If this changes climate describe any run(s) done to evaluate the new climate in enough detail that it(they) could be reproduced: N/A CAM-SIMA date used for the baseline comparison tests if different than latest: --------- Co-authored-by: Courtney Peverley <courtneyp@izumi.cgd.ucar.edu>
Update
call_command
function to include stderr output in CCPPError message.Right now, when xmllint-ing is done via
xml_tools.py
(validate_xml_file
),the output of the linter is not returned. This PR adds the stderr output
(if any) to the returned error message.
PR also decodes error messages for cleaner output.
User interface changes?: No
Testing:
test removed: N/A
unit tests: Added new doctest to test error message
system tests: N/A
manual testing: Ran with/parsed new error messages within CAM-SIMA