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.

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

Absorb GSI monitor scripts #967

Closed
WalterKolczynski-NOAA opened this issue Aug 11, 2022 · 4 comments · Fixed by #969
Closed

Absorb GSI monitor scripts #967

WalterKolczynski-NOAA opened this issue Aug 11, 2022 · 4 comments · Fixed by #969
Assignees
Labels
maintenance Regular updates and maintenance work

Comments

@WalterKolczynski-NOAA
Copy link
Contributor

Description
As part of the overarching goal of moving global workflow scripts out of component repositories and under global workflow, all of the scripts currently linked in GSI monitor should be moved to global workflow.

Requirements
All jobs, scripts, and ush scripts that are currently linked by global workflow added to the global workflow repo instead and removed from the ignore list.

Acceptance Criteria (Definition of Done)
No jobs, scripts, or ush scripts linked from GSI monitor.

Dependencies
None

@WalterKolczynski-NOAA WalterKolczynski-NOAA added the maintenance Regular updates and maintenance work label Aug 11, 2022
@WalterKolczynski-NOAA WalterKolczynski-NOAA self-assigned this Aug 11, 2022
@CoryMartin-NOAA
Copy link
Contributor

Looking ahead to when we refactor the monitoring system, I'm not sure we will want ush scripts to be kept in the global-workflow. Is that the convention that is being followed now for all components? I think, for monitoring in particular, that could get messy fast. Thoughts? @aerorahul @EdwardSafford-NOAA @ADCollard @kevindougherty-noaa

@EdwardSafford-NOAA
Copy link
Contributor

I've been viewing the monitoring refactor in terms of creating a new, wholly separate package and repo ("Monitoring 2.0") and letting the existing GSI-monitor repo sunset. At the moment only the global workflow is using GSI-monitor, so I don't see an immediate problem with this convergence of the ush. But you're right, Cory, when we get 2.0 built, putting the ush level scripts in a specific workflow could be awkward.

WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 12, 2022
Copies scripts out of the GSI monitor repo into global workflow.

Refs: NOAA-EMC#967
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 12, 2022
Updates the new GSI monitor scripts to incorporate some of the improvements
already added to other workflow scripts:
- Let preamble handle script entry/exit messages and bash set
- Replace backticks with $( ) for subshells
- Remove env prints

Refs: NOAA-EMC#967
@WalterKolczynski-NOAA
Copy link
Contributor Author

I had done the changes before these comments so I just brought the ush scripts that workflow was using over. We can move them back later if need be.

@CoryMartin-NOAA
Copy link
Contributor

My comment was more "forward looking", I have no issues with doing it for the GSI monitor.

WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/GSI-Monitor that referenced this issue Aug 12, 2022
Global workflow is moving all scripts that are only used by global workflow
currently held by other repos into global workflow.

Related workflow issue NOAA-EMC/global-workflow/pull/967
Related workflow PR NOAA-EMC/global-workflow/pull/969
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 15, 2022
`ush/radmon_ck_stdout.sh` was being ignored, but it was not even being
linked by the link script anymore, so the ignore is unnecessary.

Refs: NOAA-EMC#967
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 19, 2022
One radmon script is needed but was not previously linked, so it hadn't
been added to the repository.

Refs: NOAA-EMC#967
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 19, 2022
After moving the GSI monitor scripts to workflow, several of the scripts
would fail because set -eu is on, either due to undefined variables or
trying to operate on non-existent files. This corrects some of those issues.

Some of the non-existent files were globs, for these the bash built-in
`compgen -G` is used to check if it resolves to any files.

Refs: NOAA-EMC#967
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 19, 2022
After moving the GSI monitor scripts to workflow, several of the scripts
would fail because set -eu is on, either due to undefined variables or
trying to operate on non-existent files. This corrects some of those issues.

Some of the non-existent files were globs, for these the bash built-in
`compgen -G` is used to check if it resolves to any files.

Refs: NOAA-EMC#967
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this issue Aug 22, 2022
Removes unused mail options from radmon scripts. Also an outdated comment.

Refs: NOAA-EMC#967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Regular updates and maintenance work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants