-
Notifications
You must be signed in to change notification settings - Fork 162
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 Exception Handling Issue Found in build_all.sh #14
Comments
Implement exception handling in build_all.sh. Tested in Dell, Hera, and Cray. This improved solution will identify building error and handle exception by fail in proper message. |
Pull request initiated: #15 |
Cancelled prior pull request after request for additional changes to script. Retested after modifications were made and the changes looked good. @lgannoaa Are you ready to do the pull request again? Any further changes to make or tests to do? @EricRogers-NOAA @arunchawla-NOAA @Hang-Lei-NOAA any further feedback on the changes Lin made to build_all.sh? |
I liked what was done. If Lin makes the PR it looks good to me.
---------------------------------------------------------------
Arun Chawla
Chief
Engineering & Implementation Branch
Room 2083
National Center for Weather & Climate Prediction
5830 University Research Court
College Park, MD 20740
Phone : 301-683-3740
Mobile : 240-564-5675
Fax : 301-683-3703
------------------------------------------------------------
…On Thu, Feb 6, 2020 at 10:31 AM Kate Friedman ***@***.***> wrote:
Cancelled prior pull request after request for additional changes to
script. Retested after modifications were made and the changes looked good.
@lgannoaa <https://github.com/lgannoaa> Are you ready to do the pull
request again? Any further changes to make or tests to do?
@EricRogers-NOAA <https://github.com/EricRogers-NOAA> @arunchawla-NOAA
<https://github.com/arunchawla-NOAA> @Hang-Lei-NOAA
<https://github.com/Hang-Lei-NOAA> any further feedback on the changes
Lin made to build_all.sh?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=AL5NYI25ATTBU44CRKP6LFDRBQULNA5CNFSM4KLIIXB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7UHAY#issuecomment-582960003>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AL5NYI6VTDF2KPS5VL7W2OTRBQULNANCNFSM4KLIIXBQ>
.
|
It looks good!
Okay to be pushed into develop.
Hang
…On Thu, Feb 6, 2020 at 10:31 AM Kate Friedman ***@***.***> wrote:
Cancelled prior pull request after request for additional changes to
script. Retested after modifications were made and the changes looked good.
@lgannoaa <https://github.com/lgannoaa> Are you ready to do the pull
request again? Any further changes to make or tests to do?
@EricRogers-NOAA <https://github.com/EricRogers-NOAA> @arunchawla-NOAA
<https://github.com/arunchawla-NOAA> @Hang-Lei-NOAA
<https://github.com/Hang-Lei-NOAA> any further feedback on the changes
Lin made to build_all.sh?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=AKWSMFB3DEMIFLFAERU3NYLRBQULNA5CNFSM4KLIIXB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK7UHAY#issuecomment-582960003>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKWSMFEUHKJPQUSBHAAVEXTRBQULNANCNFSM4KLIIXBQ>
.
|
I agree with Arun and Hang. |
PR is done. Thanks everyone :-) |
Thanks Lin! I'll take a look in the morning and process it with Hang.
Kate Friedman (formerly Howard)
NOAA/NWS/NCEP/EMC Engineering and Implementation Branch
…On Thu, Feb 6, 2020 at 3:25 PM lgannoaa ***@***.***> wrote:
PR is done. Thanks everyone :-)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#14?email_source=notifications&email_token=AI7BYWMCLTMJSOC7KHD2L63RBRW4FA5CNFSM4KLIIXB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELAVABQ#issuecomment-583094278>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7BYWLSIVCGG2UKKAKDLB3RBRW4FANCNFSM4KLIIXBQ>
.
|
Submitting PR. |
Feature/chem
The rocoto_viewer script is updated to python3. In addition to making the necessary syntax updates like converting print statements into print functions (adding parentheses), this requried removing the dependency on produtils scripts (which are still in python2). This change will also make the script more portable. The largest chunk of changes to remove produtils revolves around making system calls to run rocoto. These are replaced with a simple wrapper function that calls subprocess, captures the output, and returns it as a string. This also allows for the streamlining of these portions of code. A fallback is added in case lxml is not available. This will only cause problems if lxml is not available and the user tries to read a workflow that uses external entities, in which case a message will be displayed notifying the user of the issue and providing instructions on how to install lxml with pip. A similar existing message for dateutil (used for UGCS's monthly increment) was updated similarly. Some incidental syntax updating and unused variable removal was done as they were encountered, but a more complete cleaning of these is still needed. Refs: #13, #14
All strings still using the modulo (%) operator for substitution are replaced with more readable f-strings instead. Some remaining instances that were using functions from produtil that were missed in the previous commit are updated to replace those functions. Two functions for validating a POSIX shell string are copied and updated from produtil for use here. Some minor bugs and typos are resolved. Replaced the former cPickle (which is not available for python3) calls with pickle calls. Fixed bugs in rocotocheck related to passing an int argument instead of a string to syscall and not defining current_check_time. Corrected issue with rocoto_viewer not immediately exiting when the workflow cannot be parsed (usually because lxml could not be loaded). Refs: #13, #14
…eflectivity correctly (NOAA-EMC#14)
Arun assigned Lin to fix the exception handling issues found in build_all.sh.
The text was updated successfully, but these errors were encountered: