Skip to content

pathogen-repo-ci: Multi-runtime#42

Merged
tsibley merged 6 commits intomasterfrom
trs/pathogen-repo-ci/multi-runtime
May 12, 2023
Merged

pathogen-repo-ci: Multi-runtime#42
tsibley merged 6 commits intomasterfrom
trs/pathogen-repo-ci/multi-runtime

Conversation

@tsibley
Copy link
Copy Markdown
Contributor

@tsibley tsibley commented May 11, 2023

See commit messages.

Testing

  • Checks pass

tsibley added 6 commits May 10, 2023 16:50
Not expecting any maliciousness but potentially accidental escape of the
naive single quoting.
This will let us run pathogen repo CI in both our Docker and Conda
runtimes, for example.

Docker is still the default.
Contains potentially useful debugging information.
Necessary to avoid mixing up/overwriting build results when this
workflow is invoked multiple times from the same calling workflow.
It's helpful to test that our pathogen repos work on all supported
runtimes.  This commit doesn't change the existing default of only using
the Docker runtime.  The default will be changed in a subsequent commit,
however.
This is our other major runtime, and it seems worth the extra CI time to
routinely test on it.

We may want to add the Singularity runtime at some point too, but that
would require additional runner setup work.
@tsibley
Copy link
Copy Markdown
Contributor Author

tsibley commented May 11, 2023

This might reveal bugs in our Conda runtime for ncov and seasonal-flu: nextstrain/conda-base#27 (comment)

Comment on lines +50 to +52
containing YAML. This is easily produced, for example, by pretending
you're writing normal nested YAML within a literal multi-line block
scalar (introduced by "|"):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Neat workaround for input type limitations.

@tsibley tsibley merged commit 5eebf44 into master May 12, 2023
@tsibley tsibley deleted the trs/pathogen-repo-ci/multi-runtime branch May 12, 2023 16:29
Comment on lines +106 to +108
strategy:
matrix:
runtime: ${{ fromJSON(needs.configuration.outputs.runtimes) }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thoughts on adding fail-fast: false so that runtime failures do not interfere with each other?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds like a good idea!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, that makes sense. Would you do it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

3 participants