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

Fix env_mach_specific validation #1509

Merged
merged 3 commits into from
May 10, 2017
Merged

Conversation

jedwards4b
Copy link
Contributor

@jedwards4b jedwards4b commented May 10, 2017

Fix the schema of env_mach_specific to allow environment_variables, mpirun and entry to not be specified.

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

Addresses #1505

Code review:

Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

The change to env_mach_specific.xsd fixes #1505 - thank you!

I'm not sure I see the need for the other changes here. Are there legitimate times when we expect a user to need to bypass schema validation? Or is this just a mechanism to give users a workaround in case there are not-yet-discovered problems in the xsd files? I'm wondering if it's worth cluttering the user interface with this....

I also don't love the slippery slope of introducing this new global variable, and I also notice that it makes setup_standard_logging_options a misnomer now.

All that said, I'm okay with all of these changes if you feel they're warranted.

@jgfouca
Copy link
Contributor

jgfouca commented May 10, 2017

@jedwards4b I agree with @billsacks .

Copy link
Contributor

@jgfouca jgfouca left a comment

Choose a reason for hiding this comment

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

Remove skip-validation.

@jedwards4b
Copy link
Contributor Author

I agree that the name setup_standard_logging_options is incorrect, but my intent was always that it be setup_standard_options. This option was added based on my perception of @billsacks level of distress when an xml file didn't validate, but there may be other good reasons for it. Do you have any suggestions other than the global for how to implement?

@jgfouca
Copy link
Contributor

jgfouca commented May 10, 2017

The only other way would be a very-annoying pass-down of a parameter all the way down to GenericXML.

@jedwards4b jedwards4b changed the title Allowed schema skip Fix env_mach_specific validation May 10, 2017
@jedwards4b
Copy link
Contributor Author

okay - only the schema is fixed

@jgfouca jgfouca merged commit 144cc38 into ESMCI:master May 10, 2017
@rljacob
Copy link
Member

rljacob commented Aug 2, 2017

Update the description to match the title.

@jedwards4b jedwards4b deleted the allowed_schema_skip branch August 2, 2017 13:28
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.

4 participants