Remove Direct Linking to COM from DATA for extractvars Job
#3379
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
AntonMFernando-NOAA
requested review from
JessicaMeixner-NOAA,
sbanihash and
EricSinsky-NOAA
as code owners
|
A |
There was a problem hiding this comment.
Thank you for working on this, @AntonMFernando-NOAA. There are some infile if-statements that may need some revision. Please see my comments.
ush/atmos_extractvars.sh
Outdated
| elif [[ -f "${new_infile}" ]]; then | ||
| echo "WARNING: ${new_infile} does not exist in ${subdata}. Copying skipped." |
There was a problem hiding this comment.
new_infile is defined inside the first if block, so I do not believe this elif condition will ever be met. I think you can check for the existance of new_infile immediately after the cpfs operation.
ush/atmos_extractvars.sh
Outdated
| elif [[ -f "${infile}" ]]; then | ||
| echo "WARNING: ${infile} does not exist in ${com_dir}." |
There was a problem hiding this comment.
The infile existence check has already been performed on line 74.
| elif [[ -f "${infile}" ]]; then | |
| echo "WARNING: ${infile} does not exist in ${com_dir}." | |
| else | |
| echo "WARNING: ${infile} does not exist in ${com_dir}." |
ush/atmos_extractvars.sh
Outdated
| else | ||
| echo "WARNING: ${infile} and ${new_infile} do not exist." |
There was a problem hiding this comment.
Since the existence of infile and new_infile has already been checked, this part is not needed.
| else | |
| echo "WARNING: ${infile} and ${new_infile} do not exist." |
There was a problem hiding this comment.
@EricSinsky-NOAA I’ve made the suggested changes. Please disregard my previous comments. I now understand what you meant.
ush/atmos_extractvars.sh
Outdated
| if [[ -f "${infile}" ]]; then # check if input file exists before extraction | ||
| new_infile="${outdirpre}/$(basename "${infile}")_ext" | ||
| cpfs "${infile}" "${new_infile}" | ||
| # shellcheck disable=SC2312 | ||
| ${WGRIB2} "${infile}" | grep -F -f "${varlist_d}" | ${WGRIB2} -i "${infile}" -append -grib "${outfile}" | ||
| ${WGRIB2} "${new_infile}" | grep -F -f "${varlist_d}" | ${WGRIB2} -i "${new_infile}" -append -grib "${outfile}" | ||
| elif [[ -f "${infile}" ]]; then | ||
| echo "WARNING: ${infile} does not exist in ${com_dir}." | ||
| elif [[ -f "${new_infile}" ]]; then | ||
| echo "WARNING: ${new_infile} does not exist in ${subdata}. Copying skipped." | ||
| else | ||
| echo "WARNING: ${infile} does not exist." | ||
| echo "WARNING: ${infile} and ${new_infile} do not exist." | ||
| fi |
There was a problem hiding this comment.
Please see my earlier comments. I think the same logic applies here as well.
ush/ocnice_extractvars.sh
Outdated
| elif [[ -f "${infile}" ]]; then | ||
| echo "WARNING: ${infile} does not exist in ${com_dir}." | ||
| elif [[ -f "${new_infile}" ]]; then | ||
| echo "WARNING: ${new_infile} does not exist in ${subdata}. Copying skipped." | ||
| else | ||
| echo "WARNING: ${infile} does not exist." | ||
| echo "WARNING: ${infile} and ${new_infile} do not exist." |
There was a problem hiding this comment.
Please see my earlier comments in ush/atmos_extractvars.sh. I think the same logic applies here.
ush/wave_extractvars.sh
Outdated
| elif [[ -f "${infile}" ]]; then | ||
| echo "WARNING: ${infile} does not exist in ${com_dir}." |
There was a problem hiding this comment.
infile was already checked in the previous if-block (line 31). Please see my comments in ush/atmos_extractvars.sh.
…do-NOAA/global-workflow into bug/NCO-extractvars
AntonMFernando-NOAA
added
bug
|
@EricSinsky-NOAA Removed the unnecessary |
Description
In the current GFS version, some jobs directly write to COMOUT via links in the working directories. This approach is risky, as downstream jobs might receive incomplete files and fail. In order to resolve this issue direct linking to COM from DATA should be removed. This PR addresses this issue in the extractvars job.
Resolves Remove direct linking to COM from DATA for
extractvarsjob #3376Refs [NCO Bug] Replace links with copy/move/rsync in workflow scripts #712
Refs Remove direct linking to
COMfromDATAforwavepostsbsjob #3302Refs Replace link w/ copy in the forecast job #3273
Type of change
Change characteristics
How has this been tested?
Checklist