-
Notifications
You must be signed in to change notification settings - Fork 28
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
Replace UM um_env.py configuration file with yaml file #459
Conversation
Hello @blimlim! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-07-16 05:40:11 UTC |
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 changes look good to me
I'll hold off on merging this for the moment – I just realised it might be worth adding in a depreciation error when a configuration contains a |
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.
Just a few spelling checks and a question.
Co-authored-by: Tommy Gatti <[email protected]>
Closes #454
um.py
currently sources a python fileum_env.py
in order to set environment variables by the UM, which might cause security issues. This pull request makes payu read the data from a yaml file instead.If this ends up being merged, we'll need to also convert the
um_env.py
files in the configurations to yaml.Printing out the environment through a run stage userscript shows that the relevant environment variables are the same with and without the changes:
Printed UM environment variable values without the changes:
Printed UM environment variable values with the changes:
Just to be sure, the simulation is also identical with and without the changes:
