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.

Sign up for GitHub

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

Jump to bottom

Replaces sleep with wait_for_file #2533

Closed
wants to merge 0 commits into from
Closed

Replaces sleep with wait_for_file #2533

wants to merge 0 commits into from

Conversation

HenryRWinterbottom
Copy link
Contributor

Description

This PR addresses issue #2444. The following is accomplished:

  • All sleep statements are replaced with wait_for_file for the relevant scripts beneath scripts and ush;
  • Indentation and shell-norms are updated where applicable.

Note: The WAFS scripts are not updated as per @aerorahul direction.

Resolves #2444

Type of change

  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO

How has this been tested?

The following CI tests were executed for RDHPCS Hera:

  • C48_ATM
  • C96_atm3DVar
  • C48_S2SW

The remaining configurations will be evaluated via the full CI/CD tests.

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • I have made corresponding changes to the documentation if necessary

@HenryRWinterbottom HenryRWinterbottom self-assigned this Apr 24, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 24, 2024
View reviewed changes
scripts/exgfs_atmos_grib_awips.sh Fixed Show fixed Hide fixed
scripts/exgfs_wave_nawips.sh Fixed Show fixed Hide fixed
ush/gfs_bufr.sh Outdated
exit 2
fi
done
if ! -f wait_for_file "${COM_ATMOS_HISTORY}/${RUN}.${cycle}.atm.logf${hh3}.${logfm}" "${sleep_interval}" "1000"; then

Check warning

Code scanning / shellcheck

This flag is used as a command name. Bad line break or missing [ .. ]?

This flag is used as a command name. Bad line break or missing [ .. ]?
ush/gfs_bufr_netcdf.sh Outdated
fi
done
sleep_interval=10
if ! -f wait_for_file "${COMIN}/${RUN}.${cycle}.logf${hh2}.txt" "${sleep_interval}" "1000"; then

Check warning

Code scanning / shellcheck

This flag is used as a command name. Bad line break or missing [ .. ]?

This flag is used as a command name. Bad line break or missing [ .. ]?
ush/wave_tar.sh Fixed Show fixed Hide fixed
ush/wave_tar.sh Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 24, 2024
View reviewed changes
scripts/exgfs_atmos_grib_awips.sh Outdated
done

sleep_interval=10
if [[ ! -s wait_for_file "${COM_ATMOS_GRIB_0p25}/${RUN}.${cycle}.pgrb2b.0p25.f${fcsthrs}.idx" "${sleep_interval}" "1000" ]]; then

Check notice

Code scanning / shellcheck

The mentioned syntax error was in this if expression.

The mentioned syntax error was in this if expression.
scripts/exgfs_atmos_grib_awips.sh Outdated
done

sleep_interval=10
if [[ ! -s wait_for_file "${COM_ATMOS_GRIB_0p25}/${RUN}.${cycle}.pgrb2b.0p25.f${fcsthrs}.idx" "${sleep_interval}" "1000" ]]; then

Check failure

Code scanning / shellcheck

Couldn't parse this test expression. Fix to allow more checks.

Couldn't parse this test expression. Fix to allow more checks.
scripts/exgfs_atmos_grib_awips.sh Outdated
done

sleep_interval=10
if [[ ! -s wait_for_file "${COM_ATMOS_GRIB_0p25}/${RUN}.${cycle}.pgrb2b.0p25.f${fcsthrs}.idx" "${sleep_interval}" "1000" ]]; then

Check failure

Code scanning / shellcheck

Expected test to end here (don't wrap commands in []/[[]]). Fix any mentioned problems and try again.

Expected test to end here (don't wrap commands in []/[[]]). Fix any mentioned problems and try again.
scripts/exgfs_wave_nawips.sh Outdated
err=1;export err;${errchk} || exit ${err}
fi
done
if [[ ! -s wait_for_file "${GRIBIN}" 20 "${maxtries}" ]]; then

Check notice

Code scanning / shellcheck

The mentioned syntax error was in this if expression.

The mentioned syntax error was in this if expression.
scripts/exgfs_wave_nawips.sh Outdated
err=1;export err;${errchk} || exit ${err}
fi
done
if [[ ! -s wait_for_file "${GRIBIN}" 20 "${maxtries}" ]]; then

Check failure

Code scanning / shellcheck

Couldn't parse this test expression. Fix to allow more checks.

Couldn't parse this test expression. Fix to allow more checks.
ush/gfs_bufr_netcdf.sh Fixed Show fixed Hide fixed
ush/wave_tar.sh Outdated
echo "Making TAR FILE"

alertName=$(echo $RUN|tr [a-z] [A-Z])
alertName=$(echo "${RUN}"|tr "[a-z]" "[A-Z]")

Check notice

Code scanning / shellcheck

Don't use [] around classes in tr, it replaces literal square brackets.

Don't use [] around classes in tr, it replaces literal square brackets.
ush/wave_tar.sh Outdated
echo "Making TAR FILE"

alertName=$(echo $RUN|tr [a-z] [A-Z])
alertName=$(echo "${RUN}"|tr "[a-z]" "[A-Z]")

Check notice

Code scanning / shellcheck

Don't use [] around classes in tr, it replaces literal square brackets.

Don't use [] around classes in tr, it replaces literal square brackets.
ush/wave_tar.sh Outdated
then
tardone='yes'
fi
if -f wait_for_file "${ID}.${cycle}.${type}_tar" "${sleep_interval}" "1000"; then

Check warning

Code scanning / shellcheck

This flag is used as a command name. Bad line break or missing [ .. ]?

This flag is used as a command name. Bad line break or missing [ .. ]?
ush/wave_tar.sh Outdated
tardone='yes'
fi
if -f wait_for_file "${ID}.${cycle}.${type}_tar" "${sleep_interval}" "1000"; then
tardone='yes'

Check warning

Code scanning / shellcheck

tardone appears unused. Verify use (or export if used externally).

tardone appears unused. Verify use (or export if used externally).
@HenryRWinterbottom
Copy link
Contributor Author

Please do not review until all shellnorm exceptions have been addressed.

@WalterKolczynski-NOAA
Copy link
Contributor

You shouldn't be using [[ ]] to surround the wait_for_file, just a bare function call. See those added in PR #2374.

@HenryRWinterbottom
Copy link
Contributor Author

HenryRWinterbottom commented Apr 25, 2024 •
edited
Loading

@WalterKolczynski-NOAA I hadn't been until the linter started complaining.

For example, from ush/gfs_bufr_netcdf.sh:

Error: SHELLCHECK_WARNING:
./ush/gfs_bufr_netcdf.sh:81:10: note[SC2292]: Prefer [[ ]] over [ ] for tests in Bash/Ksh.

These are littered throughout the shell linter tests.

How did you resolve this?

@WalterKolczynski-NOAA
Copy link
Contributor

@WalterKolczynski-NOAA I hadn't been until the linter started complaining.

For example, from ush/gfs_bufr_netcdf.sh:

Error: SHELLCHECK_WARNING:
./ush/gfs_bufr_netcdf.sh:81:10: note[SC2292]: Prefer [[ ]] over [ ] for tests in Bash/Ksh.

These are littered throughout the shell linter tests.

How did you resolve this?

There shouldn't be any square bracket.

@HenryRWinterbottom
Copy link
Contributor Author

@WalterKolczynski-NOAA I hadn't been until the linter started complaining.
For example, from ush/gfs_bufr_netcdf.sh:

Error: SHELLCHECK_WARNING:
./ush/gfs_bufr_netcdf.sh:81:10: note[SC2292]: Prefer [[ ]] over [ ] for tests in Bash/Ksh.

These are littered throughout the shell linter tests.
How did you resolve this?

There shouldn't be any square bracket.

I understand that. This is the statement in my bash script that is raising the exception:

# Make sure all files are available:
   sleep_interval=10
   if ! -f wait_for_file "${COMIN}/${RUN}.${cycle}.logf${hh2}.txt" "${sleep_interval}" "1000"; then
     err_exit "COULD NOT LOCATE logf${hh2} file AFTER 1 HOUR"
   fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KateFriedman-NOAA KateFriedman-NOAA Awaiting requested review from KateFriedman-NOAA

@WalterKolczynski-NOAA WalterKolczynski-NOAA Awaiting requested review from WalterKolczynski-NOAA

@aerorahul aerorahul Awaiting requested review from aerorahul

Assignees

@HenryRWinterbottom HenryRWinterbottom

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace sleep with wait_for_file()
2 participants
@HenryRWinterbottom @WalterKolczynski-NOAA