[phylo automation] run only builds for species with new data#72
[phylo automation] run only builds for species with new data#72jameshadfield wants to merge 2 commits into
Conversation
37c2bdb to
26e0306
Compare
| for access in open restricted; do | ||
| key="files/workflows/ebola/${species}/metadata_${access}.tsv.zst" |
There was a problem hiding this comment.
Should FASTA be checked too, like in many other repos?
There was a problem hiding this comment.
I thought (but should check!) that any sequence updates would get a PPX version bump and therefore a new metadata hash. Let me confirm this, otherwise yes I'll check the FASTA
There was a problem hiding this comment.
Even if that's how it's supposed to work, why not add the extra few lines of code to check anyways ("trust but verify")? head-object is cheap.
If we stick with metadata-only checks here, let's have a comment explaining why and do the same for other repos that source from Pathoplexus.
| # A workflow_dispatch forces every build to run; otherwise only run the | ||
| # builds for species whose metadata hash changed (i.e. a cache miss). | ||
| if [[ "$IS_DISPATCH" == "true" || "$BDBV_HIT" != "true" ]]; then | ||
| builds+=(bdbv/all-outbreaks bdbv/drc-uganda-2026) | ||
| fi | ||
| if [[ "$IS_DISPATCH" == "true" || "$SUDV_HIT" != "true" ]]; then | ||
| builds+=(sudv/all-outbreaks) | ||
| fi | ||
| if [[ "$IS_DISPATCH" == "true" || "$EBOV_HIT" != "true" ]]; then | ||
| builds+=(ebov/all-outbreaks) | ||
| fi |
There was a problem hiding this comment.
Hmm, I wish we didn't have to maintain this list of builds. It's easy enough to fix, but someone has to notice the automated runs didn't include the expected builds.
Slightly more complicated, but we could parse the builds in build-configs/nextstrain-automation/config.yaml and remove by species prefix. Then we'd only have to maintain the single list in the config.yaml.
There was a problem hiding this comment.
Yeah, claude had suggested this during this work but I felt it was simpler to duplicate the builds. Let me actually prototype the alternative before deciding.
The alternative is to actions/checkout in check-new-data and derive builds by filtering config.yaml's builds list by species prefix — more robust to build additions (e.g. a future bdbv/… build would be picked up automatically), but adds a checkout + a YAML parser (yq) to a job that currently checks out nothing.
There was a problem hiding this comment.
What do you think of b06f774? A little more bash magic than I'd write myself (e.g. mapfile) but I think the complexity is manageable and well documented
There was a problem hiding this comment.
Nice, seems reasonable to me. (I've only seen readarray, but it's just an alias for mapfile.)
Previously the check-new-data job only checked the ebov metadata for updates and, on any change, triggered a phylogenetic run of all builds. Now that bdbv, sudv and ebov are all uploaded by ingest, check each species independently and run only the builds for species whose data actually changed. A workflow_dispatch still forces every build to run. Relatedly, "ebov metadata" refers to two files _open and _restricted, so we get the hash for each and join them together. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
As suggested in review [1], we can avoid duplicating the builds by filtering the config-defined builds by their species prefix [1] #72 (comment)
26e0306 to
b06f774
Compare
3 participants
Previously the check-new-data job only checked the ebov metadata for updates and, on any change, triggered a phylogenetic run of all builds. Now that bdbv, sudv and ebov are all uploaded by ingest, check each species independently and run only the builds for species whose data actually changed. A workflow_dispatch still forces every build to run.
Relatedly, for each species there's now two metadata files _open and _restricted, so we get the hash for each and join them together.
Closes #71
@joverlee521 @victorlin would you mind taking a quick look? I don't think we've used this exact approach elsewhere