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

Use nextstrain/ingest #164

Merged
merged 5 commits into from
Jul 31, 2023
Merged

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Jul 31, 2023

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)

Testing

  • Ingest workflow runs locally without errors
  • Test with upload (ref)

Post-merge:

  • Slack notifications from the next run are as expected (ref).

…/vendored

subrepo:
  subdir:   "ingest/vendored"
  merged:   "9082700"
upstream:
  origin:   "https://github.com/nextstrain/ingest"
  branch:   "main"
  commit:   "9082700"
git-subrepo:
  version:  "0.4.6"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "110b9eb"
The previous commit was created by the following command:

    git subrepo clone https://github.com/nextstrain/ingest ingest/vendored

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 victorlin requested a review from a team July 31, 2023 19:07
@victorlin victorlin self-assigned this Jul 31, 2023
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.
@joverlee521
Copy link
Contributor

joverlee521 commented Jul 31, 2023

Doing a test run which will include upload but does not include Slack notifications/triggers.

Attach to test build on AWS Batch with:

nextstrain build --aws-batch --attach 46d8f038-8837-41a0-960e-1b6a307bb549 . 

Files should be uploaded to s3://nextstrain-data/files/workflows/monkeypox/branch/victorlin/centralized-ingest-git-subrepo/

Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

Thanks for starting this off @victorlin!

The test run successfully completed and all of the expected files were uploaded.

I've never figured out the best way to test Slack notifications and triggers other than ensuring the individual scripts work as expected outside the full workflow. The changes look good to me, so I'd be happy to just have this merged and monitor the automated workflows for any hiccups.

@victorlin victorlin merged commit 1ef49ba into master Jul 31, 2023
@victorlin victorlin deleted the victorlin/centralized-ingest-git-subrepo branch July 31, 2023 23:29
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.

2 participants