Skip to content

Fix unbound SED for cron commands in the launch script#606

Merged
chan-hoo merged 2 commits into
ufs-community:developfrom
chan-hoo:feature/cron_sed
Oct 7, 2021
Merged

Fix unbound SED for cron commands in the launch script#606
chan-hoo merged 2 commits into
ufs-community:developfrom
chan-hoo:feature/cron_sed

Conversation

@chan-hoo
Copy link
Copy Markdown
Collaborator

@chan-hoo chan-hoo commented Sep 23, 2021

DESCRIPTION OF CHANGES:

To fix the issue on the launch script when 'USE_CRON_TO_RELAUNCH'="TRUE", 'source_util_funcs.sh' is sourced in the script.

TESTS CONDUCTED:

WE2E tests on WCOSS_DELL_P3:

  • grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
  • grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
  • grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
  • MET_ensemble_verification

ISSUE:

Fixes issue mentioned in #605

@gsketefian
Copy link
Copy Markdown
Collaborator

@chan-hoo I think the fix you're implementing will cause the workflow to break on MACOS. We should define SED as in other scripts, which is by sourcing source_util_funcs.sh. You can do that by including the following lines in the launch script right after where scrfunc_dir is defined (I haven't debugged this though!):

homerrfs=${scrfunc_dir%/*}
ushdir="$homerrfs/ush"
. $ushdir/source_util_funcs.sh

This is the same approach as, for example, in run_WE2E_tests.sh. Thanks! @mkavulich

@chan-hoo
Copy link
Copy Markdown
Collaborator Author

chan-hoo commented Oct 6, 2021

@gsketefian , modified as suggested and tested successfully.

Copy link
Copy Markdown
Collaborator

@gsketefian gsketefian left a comment

Choose a reason for hiding this comment

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

@chan-hoo Thanks for fixing. Can you also update the PR message before merging so it doesn't say "$SED" was changed to "sed" but instead that source_util_funcs.sh was sourced? Thanks.

@mkavulich
Copy link
Copy Markdown
Collaborator

Thanks for suggesting this change @gsketefian, I can confirm this is needed for the launch script to work on MacOS (though right now we do not test this since we run the stand-alone scripts there). I am not sure why the old code passed tests before (I guess the failure was in removing old crontab entries and I didn't notice?), but this fix looks good.

@chan-hoo chan-hoo merged commit 73aadeb into ufs-community:develop Oct 7, 2021
@chan-hoo chan-hoo deleted the feature/cron_sed branch October 7, 2021 11:19
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.

3 participants