Use nextstrain/ingest (git subtree) #162
Closed
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/s3-object-exists Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/s3-object-exists - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/s3-object-exists - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/s3-object-exists - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/s3-object-exists They have the following one line difference that will be omitted in this repo: ``` @@ -1,4 +1,5 @@ #!/bin/bash +# Originally copied from nextstrain/ncov-ingest set -euo pipefail url="${1#s3://}" ```
Copied from https://github.com/nextstrain/ncov-ingest/blob/8442ba80bc2ab16c345db1ad53233542fca343fe/bin/trigger Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/trigger - https://github.com/nextstrain/forecasts-ncov/blob/b7229bad08b047d480a2b40c4bafb1e33b7fe84b/bin/trigger - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/trigger - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/trigger They are completely identical to this copy of the script.
Copied from https://github.com/nextstrain/ncov-ingest/blob/8442ba80bc2ab16c345db1ad53233542fca343fe/bin/sha256sum Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/sha256sum - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/sha256sum - https://github.com/nextstrain/forecasts-ncov/blob/b7229bad08b047d480a2b40c4bafb1e33b7fe84b/ingest/bin/sha256sum - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/sha256sum - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/sha256sum - https://github.com/nextstrain/ncov/blob/ba9953f8a61d239176e960b9b2efb925b5dff84b/scripts/sha256sum - https://github.com/nextstrain/seasonal-flu/blob/c1421ff32755a0daa24c247a8e97734a01565473/scripts/sha256sum The copis in forecasts-ncov and ncov are completely identical to this copy of the script. The other copies have the following one line difference that will be omitted in this repo: ``` @@ -1,4 +1,5 @@ #!/usr/bin/env python3 +# Originally copied from nextstrain/ncov-ingest repo """ Portable sha256sum utility. """ ```
Copied from https://github.com/nextstrain/ncov-ingest/blob/8442ba80bc2ab16c345db1ad53233542fca343fe/bin/cloudfront-invalidate Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/cloudfront-invalidate - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/cloudfront-invalidate - https://github.com/nextstrain/forecasts-ncov/blob/b7229bad08b047d480a2b40c4bafb1e33b7fe84b/ingest/bin/cloudfront-invalidate - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/cloudfront-invalidate - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/cloudfront-invalidate They are completely identical to this copy of the script.
Copied from https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/merge-user-metadata Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/merge-user-metadata - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/merge-user-metadata - https://github.com/nextstrain/zika/blob/ingest/ingest/bin/merge-user-metadata They are completely identical to this copy of the script.
Copied from https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/transform-authors Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/transform-authors - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/transform-authors - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/transform-authors They are completely identical to this copy of the script.
Copied from https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/transform-field-names Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/transform-field-names - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/transform-field-names - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/transform-field-names They are completely identical to this copy of the script.
Copied from https://github.com/nextstrain/monkeypox/blob/5c461dc7e90cd70c1f16b193f82fd1666d4c95e2/ingest/bin/transform-genbank-location Subsequent copies of this script that I checked for differences: - https://github.com/nextstrain/rsv/blob/ba171f4a43110382c38b6154be3febd50408d7bf/ingest/bin/transform-genbank-location - https://github.com/nextstrain/dengue/blob/247b2fd897361f2548627de1d97d45fae4115c5c/ingest/bin/transform-genbank-location - https://github.com/nextstrain/zika/blob/4ac8d526f9f14be10b7e8858ad469a40b72a505e/ingest/bin/transform-genbank-location They are completely identical to this copy of the script.
Add identical scripts
Add minimum CI workflow that currently only runs a shellcheck job like the shellcheck workflow in the ncov-ingest repo¹. We may choose to add more tests in the future, but shellcheck is enough for now to catch errors in bash scripts. This will be helpful as I centralize the scripts that have diverged in the various ingest workflows. ¹ https://github.com/nextstrain/ncov-ingest/blob/6fd5a9b1d87e59fab35173dbedf376632154943b/.github/workflows/shellcheck.yml
Add CI workflow
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 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
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 <13424970+victorlin@users.noreply.github.com>
Add simple Slack notifications
The previous merge commit and its attached commits were created by the
following command:
git subtree add --prefix ingest/vendored https://github.com/nextstrain/ingest HEAD
Add a section in the README on how to use this directory in the future.
Remove the copies in this repo and update references.
Remove the copies in this repo and update references. Add new positional arguments required by the centralized scripts.
victorlin
commented
Jul 29, 2023
7 tasks
joverlee521
added a commit
to nextstrain/ingest
that referenced
this pull request
Jul 31, 2023
Revisiting our previous decisions in #3 after @victorlin did more testing with the monkeypox repo with `git subtree` in nextstrain/mpox#162 and `git subrepo` in nextstrain/mpox#164.
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
Begin usage of centralized ingest scripts and add details on how to pull new updates from the central repo.
Related issue(s)
Superseded by #164.
Testing
upload/send_slack_notifications/trigger_rebuild?