-
Notifications
You must be signed in to change notification settings - Fork 212
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
make the hidden status of batch jobs xml dependent #4677
make the hidden status of batch jobs xml dependent #4677
Conversation
…ded for case.st_archive only
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.
[I think this comment is due to inadvertently typing in the comment box when trying to leave several individual comments]
CIME/utils.py
Outdated
def get_batch_script_for_job(job, hidden="True"): | ||
# this if statement is for backward compatibility | ||
if job == "case.st_archive" and not hidden: | ||
hidden = "False" |
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.
I think the new logic in this function would be easier to parse if the default for hidden
is None
and then the if
block was
if hidden is None:
if job == "case.st_archive":
hidden = False
else:
hidden = True
or, to avoid setting logical variables inside an if statement,
if hidden is None:
hidden = (job != "case.st_archive")
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 only catch is that we'd need to make sure hidden
is treated as a boolean flag if it is present in the XML file, I assume from the current logic it's read in as a string
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, and I verified that updating my env_workflow.xml
to include
diff --git a/machines/config_workflow.xml b/machines/config_workflow.xml
index 7753f0f..117b470 100644
--- a/machines/config_workflow.xml
+++ b/machines/config_workflow.xml
@@ -71,6 +71,7 @@
<workflow_jobs id="case.cupid" prepend="default">
<job name="case.cupid">
<template>template.cupid</template>
+ <hidden>False</hidden>
<dependency>case.st_archive</dependency>
<prereq>1</prereq>
<runtime_parameters MACH="derecho">
works as expected. Thanks!
if ( | ||
(job != "case.st_archive" and hidden is None) | ||
or hidden == "True" | ||
or hidden == "true" | ||
): | ||
self._hidden_batch_script[job] = True | ||
else: | ||
self._hidden_batch_script[job] = False |
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.
How about
self._hidden_batch_script[job] = ((hidden is None and job != "case.st_archive") or (hidden is not None and hidden.lower() == "true"))
(The hidden is not None
is needed because AttributeError: 'NoneType' object has no attribute 'lower'
)
instead of the if statement?
Currently this is harded for case.st_archive only. This change
makes the hidden status of job scripts an xml variable in the config_workflow.xml file.
The default is hidden and case.st_archive is still an exception so this change is backward
compatible.
Test suite: scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes
User interface changes?:
Update gh-pages html (Y/N)?: