UW-255: Changes needed for SRW fill_jinja_template replacement.#237
Conversation
WeirAE
left a comment
There was a problem hiding this comment.
Tests clear, good change, just checking one thing - is the log entry in SRW not capturing the script name because of our log name change or is the SRW script not capturing the child call? We may want to change other logging to ensure the entries are still showing the correct script
|
@WeirAE SRW doesn't really have consistent logging throughout. There is a logger at the top level of the generate workflow script, but it isn't named or passed to any of the calls to other functions. There are no classes currently, so we're in a bit of a gray area. In this way, we can have the output that is sent to either a basic bash log, or a python log show up with some additional information. For example, this function is called separate three times for three separate config files from the bash run script for the forecast model. |
WeirAE
left a comment
There was a problem hiding this comment.
Should be fine for now, we can revisit taking over logging for other apps later. LGTM
|
@venitahagerty Did I address your concerns? Could I do more? |
|
I'm good |
Description
While integrating uwtools into SRW, a few changes were needed.
In addition, running the tests on Hera highlighted that we don't yet have jsonschema installed/supported there, so we should skip that test when the package isn't available.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
I ran the linter and pytests manually on hera.
Checklist