search

nextstrain / ncov-ingest

A pipeline that ingests SARS-CoV-2 (i.e. nCoV) data from GISAID and Genbank, transforms it, stores it on S3, and triggers Nextstrain nCoV rebuilds.
MIT License
36 stars 20 forks source link

Overhaul GH Action workflows #440

Closed joverlee521 closed 8 months ago

joverlee521 commented 9 months ago

Description of proposed changes

Overhaul the GH Action workflows with the main goal to use the shared pathogen-repo-build workflow. This allows the GH Action workflow success/failure status to match the AWS Batch job status and will allow us to monitor the success/failure of automated workflows via the Pathogen workflow status page.

I made other improvements to use workflow inputs in order to reduce the 8 different GH Action workflows down to 2 workflows. See commits for details.

Related issue(s)

Related to https://github.com/nextstrain/.github/issues/46

Checklist

joverlee521 commented 9 months ago

The test GISAID build completed successfully:

It still fetched from the upstream database when the fetch_from_database input was false, but I believe this should be fixed with 4d5b6113dc2b6a7c00d340c482b0b992dd1e99eb.

The test GenBank build failed during upload due to permission issues because I forgot the NextstrainNcovIngestUploader policies allow the "files/ncov/open/" prefix (not "files/workflows/ncov/open/").

I'll update the trial prefix to "files/ncov/open/trial//" in order to match the production prefix. I don't think we will ever want to update the ncov production URLs since they are advertised in our SARS-CoV-2 docs.

joverlee521 commented 9 months ago

Updated GenBank trial prefix in force-push

Running a new GenBank trial build.

joverlee521 commented 9 months ago

GenBank trial build completed successfully and uploaded outputs to s3://nextstrain-data/files/ncov/open/trial/overhaul-gh-action-workflows/.

I'm doing one final trial run today to test the fetch_from_database input which should be fixed by 4c15c84539157db5f09366033050cacfdcc118fc. If all goes well, I'll merge this on Monday so that I can monitor the automated runs.

joverlee521 commented 9 months ago

Okay, actual final trial run with final fixes to fetch_from_database in b926bd15dedc13e59d6bd36d9a71b5823d27858a 🤞

joverlee521 commented 8 months ago

Ah, missed today's automated runs. I'll merge EOD today in case there are other comments.

corneliusroemer commented 8 months ago

Thanks for cleaning up the workflows @joverlee521!

I just tried this out and it took a bit of time to understand how to configure the workflow input params to get it to do what I want (run on branch, do a test run, no fetch).

Maybe there's scope for a single "test" workflow that simply does repository dispatch with these params? The new single workflow is great for maintenance but might cause errors when misused due to complexity in selecting the right inputs. A neutered test dispatch might reduce accidental misuse likelihood.

corneliusroemer commented 8 months ago

Oh wow, I just discovered that the logs are streamed from AWS batch - that's amazing! Great job @joverlee521!

image
joverlee521 commented 8 months ago

Oh wow, I just discovered that the logs are streamed from AWS batch - that's amazing!

This is all thanks to @tsibley's update to the pathogen-repo-build workflow to let it stay attached to the AWS Batch jobs 🙌

joverlee521 commented 8 months ago

I just tried this out and it took a bit of time to understand how to configure the workflow input params to get it to do what I want (run on branch, do a test run, no fetch).

Maybe there's scope for a single "test" workflow that simply does repository dispatch with these params? The new single workflow is great for maintenance but might cause errors when misused due to complexity in selecting the right inputs. A neutered test dispatch might reduce accidental misuse likelihood.

Are there additional docs that I can add to clarify the use of the workflow input params?

If we wanted to support repository dispatch, we would have to complicate the workflows a bit to check for github.event.client_payload.<input_name> params in addition to the input.<input_name> checks. I'm not sure that's worth adding right now. I'm not too worried about "misuse" since cancelling the GitHub workflow run will also cancel the attached AWS Batch job now.