Skip to content
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

More early resolve issues #414

Merged
merged 8 commits into from
Aug 17, 2016
Merged

Conversation

jedwards4b
Copy link
Contributor

@jedwards4b jedwards4b commented Aug 17, 2016

This change unresolves some variables in the case that were being resolved too early, this resulted in some of the perl code breaking which I've put a bandaid on in the hope that it will be replaced soon.

Test suite: scripts_regression_tests
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #406

User interface changes?:

Code review:

@@ -424,7 +424,8 @@

<batch_system MACH="yellowstone" type="lsf" version="9.1">
<queues>
<queue walltimemax="24:00" jobmin="1" jobmax="8">caldera</queue>
<queue walltimemax="24:00" jobmin="1" jobmax="1" jobname="case.lt_archive">hpss</queue>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a jobname attribute to the queue node. This allows me to force the case.lt_archive script to use the hpss queue on yellowstone.

@rljacob
Copy link
Member

rljacob commented Aug 17, 2016

Does this fix #406 or just address part of it?

@@ -330,6 +330,8 @@ def get_submit_args(self, case, job):
if name is None:
submitargs+=" %s"%flag
else:
if name.startswith("$"):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In config_batch.xml the submit_args node references other xml variables, we need to strip the $ from the front of the variable so that we can resolve it. We have to do this explicitly because this is an attribute not a node,

@jedwards4b
Copy link
Contributor Author

I believe that it fixes 406

@jgfouca
Copy link
Contributor

jgfouca commented Aug 17, 2016

This all looks fine. I could swear some of these exact same changes were merged yesterday though, so that confuses me.

@jedwards4b
Copy link
Contributor Author

They were merged Monday - but I found several failing tests related to that perl build-namelist so I reverted the merge and fixed those issues before issuing the PR again. Sorry for the confusion.

@jgfouca
Copy link
Contributor

jgfouca commented Aug 17, 2016

Ah, forgot about that, thanks.

@jgfouca jgfouca merged commit 196c7dc into ESMCI:master Aug 17, 2016
@jedwards4b jedwards4b deleted the more_early_resolve_issues branch August 23, 2016 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some vars resolve too early
3 participants