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

Should the pathogen-repo-ci be louder if no workflows are run? #92

Closed
2 tasks done
joverlee521 opened this issue Jun 10, 2024 · 7 comments
Closed
2 tasks done

Should the pathogen-repo-ci be louder if no workflows are run? #92

joverlee521 opened this issue Jun 10, 2024 · 7 comments
Assignees

Comments

@joverlee521
Copy link
Contributor

joverlee521 commented Jun 10, 2024

TODO

  • move nextstrain-pathogen.yaml file checks out into a single step prior to workflow run steps
  • add aggregate success step, modeled on pathogen-repo-build wait-conclusion logic to trigger failure when no build steps are run

Original issue content

Follow up to #89

Just noticed in our docker-base CI workflow that all the pathogen-repo-ci jobs have "succeeded" even though none of them ran any workflows.

This outcome is pretty misleading and can potentially lead to us missing when changes actually break pathogen workflows if we don't notice that the workflows never actually ran within CI.

@joverlee521
Copy link
Contributor Author

I think fine to leave each ingest/phylogenetic/nextclade workflow as optional, but maybe we should have a final conclusion job that checks at least one of the workflows ran?

@genehack
Copy link
Contributor

I think fine to leave each ingest/phylogenetic/nextclade workflow as optional, but maybe we should have a final conclusion job that checks at least one of the workflows ran?

In this particular case, every repo where the pathogen-repo-ci workflow is being used lacks the nextstrain-pathogen.yaml file in the repo root.

Maybe instead of testing for the presence of that file in the individual build steps, and skipping if it's absent, there should be a preliminary job that checks once, and generates a failure (e.g., exit 1 or the like) if the file is missing.

@joverlee521
Copy link
Contributor Author

In this particular case, every repo where the pathogen-repo-ci workflow is being used lacks the nextstrain-pathogen.yaml file in the repo root.

Maybe instead of testing for the presence of that file in the individual build steps, and skipping if it's absent, there should be a preliminary job that checks once, and generates a failure (e.g., exit 1 or the like) if the file is missing.

Yup, definitely support erroring early on this! In addition, I still think it'd be helpful to fail when none of the expected workflows ran.

@genehack
Copy link
Contributor

Yup, definitely support erroring early on this! In addition, I still think it'd be helpful to fail when none of the expected workflows ran.

You wouldn't happen to be able to point me to an example of that sort of aggregated failure check in a GitHub Action, would you?

@joverlee521
Copy link
Contributor Author

You wouldn't happen to be able to point me to an example of that sort of aggregated failure check in a GitHub Action, would you?

I think the wait-conclusion job in the pathogen-repo-build workflow is a good example:

# Since the wait-N jobs use "continue-on-error: true" out of necessity (to
# avoid failing the whole workflow when they time out and get cancelled), we
# use a final job here to succeed or fail the whole workflow based on the
# aggregate of their "attach" step conclusions.
wait-conclusion:
needs: [wait-1, wait-2, wait-3, wait-4]
if: always()
runs-on: ubuntu-latest
steps:
- name: All attach steps in wait-N jobs were successful (or skipped)
run: |
# shellcheck disable=SC2242
exit ${{ contains(needs.*.outputs.attach-step-conclusion, 'failure') && '1' || '0' }}
# XXX TODO: Jobs can fall off the end of our wait-N chain and appear to be
# successful/complete in GitHub but still running on AWS. Probably very
# rare in reality, though, for an AWS job to take longer than 24h?
# -trs, 12 Sept 2023

@genehack
Copy link
Contributor

You wouldn't happen to be able to point me to an example of that sort of aggregated failure check in a GitHub Action, would you?

I think the wait-conclusion job in the pathogen-repo-build workflow is a good example:

Ah, yeah, cool, I can do the couple changes here.

@genehack genehack self-assigned this Jun 11, 2024
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 11, 2024
This is testing an outdated scenario — the `pathogen-repo-ci` workflow
no longer cares whether you have example data or not; only that you
have a Snakefile and a CI-specific build config in the right spot.
genehack added a commit that referenced this issue Jun 12, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 12, 2024
This is testing an outdated scenario — the `pathogen-repo-ci` workflow
no longer cares whether you have example data or not; only that you
have a Snakefile and a CI-specific build config in the right spot.
genehack added a commit that referenced this issue Jun 13, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 13, 2024
This is testing an outdated scenario — the `pathogen-repo-ci` workflow
no longer cares whether you have example data or not; only that you
have a Snakefile and a CI-specific build config in the right spot.
genehack added a commit that referenced this issue Jun 13, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 13, 2024
This is testing an outdated scenario — the `pathogen-repo-ci` workflow
no longer cares whether you have example data or not; only that you
have a Snakefile and a CI-specific build config in the right spot.
genehack added a commit that referenced this issue Jun 13, 2024
* Add ids to build steps in `pathogen-repo-ci`
* Add `run-attempted` output to `run-nextstrain-ci-build`
* Set output to true or false depending on whether build was attempted
* Add step to `pathogen-repo-ci` to read outputs from
  `run-nextstrain-ci-build` and validate at least one build was tried
genehack added a commit that referenced this issue Jun 13, 2024
This is testing an outdated scenario — the `pathogen-repo-ci` workflow
no longer cares whether you have example data or not; only that you
have a Snakefile and a CI-specific build config in the right spot.
@joverlee521
Copy link
Contributor Author

Resolved by a combination of #95, #96, #98, and #99.

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

No branches or pull requests

2 participants