Add simple Slack notifications #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Copied from https://github.com/nextstrain/ncov-ingest/blob/8442ba80bc2ab16c345db1ad53233542fca343fe/bin/notify-slack Subsequent copies of this script that are functionally identical: - https://github.com/nextstrain/forecasts-ncov/blob/b7229bad08b047d480a2b40c4bafb1e33b7fe84b/ingest/bin/notify-slack - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/notify-slack There was a one line difference in the rsv script that will be omitted in this repo: ``` @@ -1,4 +1,5 @@ #!/bin/bash +# Originally copied from nextstrain/ncov-ingest repo set -euo pipefail : "${SLACK_TOKEN:?The SLACK_TOKEN environment variable is required.}" ```
Changes copied from nextstrain/mpox@8757890 Subsequent copies of this script that contain identical changes: - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/notify-slack - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/notify-slack These changes use optional args so they will not require changes to calls of the previous version of the script in ncov-ingest, forecasts-ncov, and rsv.
Copied from https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/notify-on-job-start I decided to copy the version in monkeypox because it is easier to update to be a generalized script than the ncov-ingest version which has unique changes to support the run-nextclade-full scripts.¹ ¹ nextstrain/ncov-ingest@88be153...e898400
Generalize the script by using new args to customize the Slack message and point to the appropriate GitHub Action URL. These changes were made based on diffs with subsequent copies of the script that edited the Slack message: - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/notify-on-job-start - https://github.com/nextstrain/forecasts-ncov/blob/70bf78f459a3706dd817ae5f711af3b74887d7b1/bin/notify-on-job-start - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/notify-on-job-start This will require all calls of the script to be updated since job_name and repo_name are required args.
The default value is "ingest" which is the expected ingest directory for standard pathogen repos. Adding the optional arg to support customizations for historical repos such as ncov-ingest that do not follow the new standardized repo structure.
Copied from https://github.com/nextstrain/ncov-ingest/blob/6fd5a9b1d87e59fab35173dbedf376632154943b/bin/notify-on-job-fail The following commits will update and generalize the script to incorporate changes made subsequent copies.
Follow patterns set in notify-on-job-start where the environment variables `AWS_BATCH_JOB_ID` and `GITHUB_RUN_ID` are used directly instead of reassigning them to local variables and use double square brackets.
Generalize the script by using new args to customize the Slack message and point to the appropriate GitHub Action URL. The repo_name arg was added based on diffs with subsequent copies of the script that edited the Slack message: - https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/notify-on-job-fail - https://github.com/nextstrain/forecasts-ncov/blob/70bf78f459a3706dd817ae5f711af3b74887d7b1/ingest/bin/notify-on-job-fail - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/notify-on-job-fail - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/notify-on-job-fail - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/notify-on-job-fail Although the job_name arg is not necessary based on diffs with subsequent copies of the script, it's nice to be able to customize the Slack message for different ingest jobs. It's also a plus to mirror the args for notify-on-job-start. This will require all calls of the script to be update since job_name and repo_name are required args.
I realized in my testing of the script that broadcasting does not work for file uploads. Confirmed that this is not an argument for the files.upload API.¹ Including the argument did not cause an error, but better to remove it so it doesn't confuse us in the future. ¹ https://api.slack.com/methods/files.upload
victorlin
approved these changes
Jul 24, 2023
Using GitHub repos by org/name pair to make scripts more generalizable. This will also work really well with the `GITHUB_REPOSITORY` variable available for GitHub Action workflows. Co-authored-by: Victor Lin <[email protected]>
22 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
Adds the Slack notification scripts
Copied existing versions from ncov-ingest or monkeypox then edited in subsequent commits to generalize the scripts.
See commits for details.
There are other notify-* scripts that have been excluded from this PR because they rely on S3 interaction scripts. They will be added in a separate PR.
Related issue(s)
Subset of scripts listed in #1
Testing