-
Notifications
You must be signed in to change notification settings - Fork 119
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
[develop] Add job cards for wrappers for individual machines and tasks #924
[develop] Add job cards for wrappers for individual machines and tasks #924
Conversation
For the sake of further discussion I'll echo my points from today's code management meeting as to why job cards should not be included in the repository:
@RatkoVasic-NOAA I want to be clear that I understand the frustrations of debugging the workflow when a task fails. I do not want to stand in the way of development: I want to make debugging as easy as possible! I agree it would be more convenient if we had a tool that could just spit out a job card without needing to run rocoto as an intermediary. But for the reasons above I argue that putting these job cards in the repository will do more harm than good, especially in the long run! I can think of some potential solutions/mitigations for the difficulties you are having in lieu of this PR:
I hope others can chime in with other helpful features and potential solutions. And of course I welcome further discussion and rebuttals, I am by no means the final authority on this. |
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.
Thank you very much! Once @mkavulich gives his approval, I will be able to submit the Jenkins 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.
Thanks again @RatkoVasic-NOAA for making the suggested changes. I had a few more suggestions that are minor (since the same comment appears in all files it shows up as a lot of suggested changes). Not mandatory changes, but I thought this would clarify the text a little.
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
Co-authored-by: Michael Kavulich <[email protected]>
@mkavulich thanks for corrections! |
Thanks @mkavulich and @RatkoVasic-NOAA! I will go ahead and launch the Jenkins tests now. |
The
The WE2E Coverage tests were manually submitted on Derecho and all successfully passed:
|
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.
Adding my official approval for the record
The Since this PR doesn't affect the code in the SRW App, Hera being down for maintenance today (12/05/2023), and with PR #977 already in place to correct these issues, I will go ahead and merge this work now. |
DESCRIPTION OF CHANGES:
Added job cards for individual tasks, similar to wrapper scripts, but tailored for each machine.
Type of change
TESTS CONDUCTED:
DOCUMENTATION:
@gspetro-NOAA we can add few lines to document this.
ISSUE:
#909
CHECKLIST
LABELS (optional):
A Code Manager needs to add the following labels to this PR:
Contributors:
@EdwardSnyder-NOAA
@natalie-perlin