From b2153859776ddcba13144104fd1d8ede4088ef2a Mon Sep 17 00:00:00 2001 From: Thomas Sibley Date: Wed, 12 Jun 2024 11:49:04 -0700 Subject: [PATCH] pathogen-repo-ci: Don't run workflow build steps if they're not going to do anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of checking for whether to do anything or not _inside_ each build step individually, move the check to the step's condition. This will make the logs a lot clearer and mean the GitHub workflow metadata stays more in sync with reality. I've used hashFiles() to check for file existence—it returns the empty string, a falsey value, when there are no matching files—but we could swap that out for using the output of a prior setup step that runs some shell to determine what to run and what to skip. Doing so might make more sense if the conditional becomes more complicated or we want to do more detailed reporting on _why_ steps were skipped or not. With the changes, the internal-to-this-workflow action, run-nextstrain-ci-build, is no longer that useful and so I've removed it in favor of inlining things. I think this is an improvement for readability. Related-to: --- .github/workflows/pathogen-repo-ci.yaml | 63 ++++++++++++------ README.md | 1 - actions/run-nextstrain-ci-build/action.yaml | 71 --------------------- 3 files changed, 42 insertions(+), 93 deletions(-) delete mode 100644 actions/run-nextstrain-ci-build/action.yaml diff --git a/.github/workflows/pathogen-repo-ci.yaml b/.github/workflows/pathogen-repo-ci.yaml index 6f25e33..bde1b53 100644 --- a/.github/workflows/pathogen-repo-ci.yaml +++ b/.github/workflows/pathogen-repo-ci.yaml @@ -75,7 +75,7 @@ on: artifact-name: description: >- A base name to use for the uploaded artifacts from the - build. This will be concatenated with the directory name + build. This will be concatenated with the runtime name used to invoke the build to generate the full artifact file name. @@ -252,35 +252,56 @@ jobs: runtime: ${{ matrix.runtime }} - name: Run ingest + if: hashFiles('ingest/Snakefile', 'ingest/build-configs/ci/config.yaml') id: ingest - uses: ./.git/nextstrain/.github/actions/run-nextstrain-ci-build - with: - directory: ingest - runtime: ${{ matrix.runtime }} - artifact-name: ${{ inputs.artifact-name }} + run: nextstrain build ingest --configfile build-configs/ci/config.yaml - name: Run phylogenetic + if: hashFiles('phylogenetic/Snakefile', 'phylogenetic/build-configs/ci/config.yaml') id: phylogenetic - uses: ./.git/nextstrain/.github/actions/run-nextstrain-ci-build - with: - directory: phylogenetic - runtime: ${{ matrix.runtime }} - artifact-name: ${{ inputs.artifact-name }} + run: nextstrain build phylogenetic --configfile build-configs/ci/config.yaml - name: Run nextclade + if: hashFiles('nextclade/Snakefile', 'nextclade/build-configs/ci/config.yaml') id: nextclade - uses: ./.git/nextstrain/.github/actions/run-nextstrain-ci-build + run: nextstrain build nextclade --configfile build-configs/ci/config.yaml + + - if: always() + uses: actions/upload-artifact@v4 with: - directory: nextclade - runtime: ${{ matrix.runtime }} - artifact-name: ${{ inputs.artifact-name }} + name: ${{ inputs.artifact-name }}-${{ inputs.runtime }} + if-no-files-found: ignore + # @actions/glob has no support for brace expansion. Hrumph. + path: | + ingest/.snakemake/log/ + ingest/auspice/ + ingest/benchmarks/ + ingest/logs/ + ingest/results/ + + phylogenetic/.snakemake/log/ + phylogenetic/auspice/ + phylogenetic/benchmarks/ + phylogenetic/logs/ + phylogenetic/results/ + + nextclade/.snakemake/log/ + nextclade/auspice/ + nextclade/benchmarks/ + nextclade/logs/ + nextclade/results/ - name: Verify a workflow ran + env: + ingest: ${{ steps.ingest.conclusion }} + phylogenetic: ${{ steps.phylogenetic.conclusion }} + nextclade: ${{ steps.nextclade.conclusion }} run: | - # shellcheck disable=SC2242 + # Show step conclusions. + echo "ingest $ingest" + echo "phylogenetic $phylogenetic" + echo "nextclade $nextclade" - # if we see at least one success, we're good - echo "INGEST ATTEMPTED=${{ steps.ingest.outputs.run-attempted }}" - echo "PHYLOGENETIC ATTEMPTED=${{ steps.phylogenetic.outputs.run-attempted }}" - echo "NEXTCLADE ATTEMPTED=${{ steps.nextclade.outputs.run-attempted }}" - exit ${{ contains(steps.*.outputs.run-attempted, 'true') && '0' || '1' }} + # Assert status; if we see at least one attempt, regardless of + # success/failure, we're good. + [[ $ingest != skipped || $phylogenetic != skipped || $nextclade != skipped ]] diff --git a/README.md b/README.md index b9a506a..261e48e 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,6 @@ See also GitHub's [documentation on issue and PR templates](https://docs.github. Invoked by our GitHub Actions workflows, including the reusable workflows below. -- [Run Nextstrain CI Build](actions/run-nextstrain-ci-build/action.yaml) - [Setup Nextstrain CLI](actions/setup-nextstrain-cli/action.yaml) - [shellcheck](actions/shellcheck/action.yaml) - [Setup SSH](actions/setup-ssh/action.yaml) access to runner machine diff --git a/actions/run-nextstrain-ci-build/action.yaml b/actions/run-nextstrain-ci-build/action.yaml deleted file mode 100644 index 2f4a1bb..0000000 --- a/actions/run-nextstrain-ci-build/action.yaml +++ /dev/null @@ -1,71 +0,0 @@ -name: run-nextstrain-ci-build -description: >- - Runs a single `nextstrain build` command in a given sub-directory of - a pathogen repo. Must be provided with the name of the sub-directory - and the runtime to use. Requires that the Nextstrain CLI runtime - already be provisioned (e.g., via the `setup-nextstrain-cli` action - in this repo). - - Note that this action exists primarily as a means to keep the - `pathogen-repo-ci` workflow DRY; it is unlikely to be useful outside - the context of that specific workflow. - -inputs: - artifact-name: - description: >- - Name to append to the build directory to generate a - unique artifact name for the upload action. - type: string - required: true - directory: - description: The name of the sub-directory to run the build from - type: string - required: true - runtime: - description: Nextstrain runtime to use for the build - type: string - required: true - -outputs: - run-attempted: - description: >- - Boolean indicating if the build step was _attempted_. - - N.b., this does not indicate if the build step *succeeded*, only - that the requirements were met to attempt to run it. - value: ${{ steps.run-build.outputs.run-attempted }} - -runs: - using: "composite" - steps: - - id: run-build - env: - DIR: ${{ inputs.directory }} - run: | - if [[ -f "$DIR"/Snakefile && -f "$DIR"/build-configs/ci/config.yaml ]]; then - echo "run-attempted=true" >> "$GITHUB_OUTPUT" - - nextstrain check-setup ${{ inputs.runtime }} --set-default - nextstrain build "$DIR" --configfile build-configs/ci/config.yaml - else - echo "run-attempted=false" >> "$GITHUB_OUTPUT" - - echo "Skipping $DIR build due to one or more missing files." - for i in "$DIR"/Snakefile "$DIR"/build-configs/ci/config.yaml; do - [[ -f $i ]] || echo missing "$i" - done - fi - shell: bash - - - id: upload-artifact - if: always() - uses: actions/upload-artifact@v4 - with: - name: ${{ inputs.artifact-name }}-${{ inputs.directory }}-${{ inputs.runtime }} - if-no-files-found: ignore - path: | - ${{ inputs.directory }}/.snakemake/log/ - ${{ inputs.directory }}/auspice/ - ${{ inputs.directory }}/benchmarks/ - ${{ inputs.directory }}/logs/ - ${{ inputs.directory }}/results/