-
Notifications
You must be signed in to change notification settings - Fork 32
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
Change $(foo) to {{ foo }} in YAMLs #928
Conversation
The |
…SApp into feature/yaml_syntax
@RussTreadon-NOAA @guillaumevernieres I'm removing a couple of tests that seem more trouble than they're worth. Please let me know if you think I should try to make them work. I'm of the opinion that the EWOK to GDAS YAML tool needs to be something a bit more comprehensive to be useful, and the Aero YAML generator should be tested through the workflow tests. |
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, ChatGPT.
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 tile for this PR is simple. The amount of work involved is far from simple. Looks good to me. Approve pending Passed from CI tests.
I think the CI kicked off before I removed these annoying tests, so we need to verify that if there are failures that they are just the 2 I have since removed. If other failures, I'll investigate next week. |
ac5c801
Automated Global-Workflow GDASApp Testing Results:
|
Hera is now 2/3 Rocky 8. Logins automatically take users to Rocky 8 nodes. The following g-w components failed to build
These components were successfully built
g-w issue #2329 documents the transition of g-w components to Hera Rocky 8. |
Manually copy changes from PR #981 into working copy of feature/yaml_syntax on Hera. Rerun
Rerun
Do we need to update the test reference to account for the move to Hera Rocky 8? Rerun sequence of test_gdasapp_soca jobs leading up to
What library or module do we need to load so that |
I think |
Loading |
@RussTreadon-NOAA the ctest pythonpaths are set at compile time. So if they are just set and then the test is re-ran, that won't help. Sorry if that wasn't clear! |
Is a simple |
I think it has to be cleaned, or at least the cmake cache needs cleaned (not 100% sure how to do this) because the cmake sets the PYTHONPATH. |
|
Rebuilt from the top.
The above is taken from I'll hit pause on this for the time being. |
@RussTreadon-NOAA this is odd, this PR now no longer modifies soca files (I thought) |
Agreed. The observed behavior is odd. @guillaumevernieres or @AndrewEichmann-NOAA: have either of you recently run GDASApp g-w ctests using |
It's fine in containers, we'll test on hpc. |
Installed GDASApp
Repeat the above test on Orion but install GDASApp
Note that It's interesting that GDASApp PR #981 fixes the Based on the above the |
Build
Will trigger orion-GW-RT. hera-GW-RT still fails while building some g-w components. One lingering question is whether or not to include the new file |
Automated Global-Workflow GDASApp Testing Results:
|
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.
g-w ctests pass on Orion. Whether or not we merge ush/modify_yaml_syntax.py
into develop
is an open question. We can merge it in now if we think others will benefit from it as they update yamls. It can be removed at a later date. Alternatively, we remove it now.
Approve.
@guillaumevernieres , any thoughts about including or removing the utility script, This script replaces the old template format with the new template format. Several |
@RussTreadon-NOAA , we can keep it until we're done modifying our templates. Thanks for the heads up again. |
OK. I'll merge this PR into |
To be consistent throughout GFS/GDAS, we should use
{{ foo }}
syntax whenever possible in our templates.Succumbing to peer pressure, ChatGPT helped me write a quick script to do this :-)