Skip to content

Conversation

@joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Sep 10, 2025

Description of proposed changes

Originally implemented as part of #462. Pulled out as a separate PR since other changes in the original PR are being discussed.

NOTE: This is changing the schema for the workflow registration originally added (but not yet released) in #461.

List workflows under a top level workflows key in the nextstrain-pathogen.yaml file where each workflow has it's own compatibility key:

workflows:
  ingest:
    compatibility:
      nextstrain run: True
  phylogenetic:
    compatibility:
      nextstrain run: True

The top level compatibility['nextstrain run'] boolean is still supported for backwards compatibility. Dropped support for top level compatibility declaration as discussed.

Checklist

  • Checks pass
  • Update changelog

Post-merge/release

@joverlee521 joverlee521 force-pushed the workflow-compatibility branch from d117abe to e701c24 Compare September 15, 2025 22:09
@tsibley tsibley self-requested a review September 17, 2025 23:02
joverlee521 added a commit to nextstrain/pathogen-repo-guide that referenced this pull request Sep 23, 2025
Workflow changes to support running the workflows with the 
`nextstrain run` command.

I specifically chose _not_ to declare compatibility for `nextstrain run` in 
the `nextstrain-pathogen.yaml` because the phylogenetic and nextclade workflows 
are incomplete in this guide. Once workflow specific compatibility is 
supported,¹ we can declare compatibility for the ingest workflow.

¹ <nextstrain/cli#472>
Copy link
Contributor

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

I continue to +1 for the change in nextstrain-pathogen.yaml! My comments here are almost all around the way workflows are exposed in Nextstrain CLI. Happy to discuss my suggestions if you want to consider them. But also, if you are happy not to care about the details, I can make the changes as I see fit instead of going back and forth here.

joverlee521 added a commit that referenced this pull request Sep 23, 2025
joverlee521 added a commit that referenced this pull request Sep 23, 2025
Declutter the version output for pathogen worklfows but still keeping a simple 
list of pathogen workflow names for discoverability.
 
Based on feedback 
<#472 (comment)>
joverlee521 added a commit to nextstrain/zika that referenced this pull request Sep 23, 2025
joverlee521 added a commit to nextstrain/measles that referenced this pull request Sep 23, 2025
joverlee521 added a commit to nextstrain/mumps that referenced this pull request Sep 23, 2025
tsibley added a commit that referenced this pull request Sep 24, 2025
…run` compatibility

Workflows are now registered under a top level "workflows" key in the
nextstrain-pathogen.yaml file, and each workflow has its own
"compatibility" declarations.  The only current "compatibility"
declaration remains "nextstrain run".

This allows for more meta information about each workflow to be
registered in one place not under the banner of compatibility.

The nextstrain-pathogen.yaml schema changes to a workflow-first
structure like:

  workflows:
    ingest:
      compatibility:
        nextstrain run: yes
    phylogenetic:
      compatibility:
        nextstrain run: no

instead of a compatibility-first structure like:

  compatibility:
    nextstrain run:
      ingest: yes
      phylogenetic: no

as previously introduced¹ (but not yet released).  As it was never
released, don't bother supporting the latter schema at all anymore.

Change first arose out of discussion started by @victorlin in a related
PR.²  Subsequent discussion with @tsibley³ led to dropping support for
the unreleased schema.

¹ <#461>
² <#462 (comment)>
³ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
tsibley added a commit that referenced this pull request Sep 24, 2025
List the workflow names on the same line as the pathogen name@version
when there are any workflows and emit nothing when there aren't.  This
fits more in the style of the rest of `nextstrain version` and is much
less cluttered (more information dense), particularly considering that
we, in short order too, expect ~all of our pathogens (and all their
future versions) to have the same 2–3 workflows listed (nextclade,
ingest, phylogenetic).

Prompted by feedback from @tsibley.¹

¹ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
@tsibley tsibley force-pushed the workflow-compatibility branch 2 times, most recently from fad921c to 8e98a95 Compare September 24, 2025 21:16
tsibley added a commit that referenced this pull request Sep 24, 2025
…run` compatibility

Workflows are now registered under a top level "workflows" key in the
nextstrain-pathogen.yaml file, and each workflow has its own
"compatibility" declarations.  The only current "compatibility"
declaration remains "nextstrain run".

This allows for more meta information about each workflow to be
registered in one place not under the banner of compatibility.

The nextstrain-pathogen.yaml schema changes to a workflow-first
structure like:

  workflows:
    ingest:
      compatibility:
        nextstrain run: yes
    phylogenetic:
      compatibility:
        nextstrain run: no

instead of a compatibility-first structure like:

  compatibility:
    nextstrain run:
      ingest: yes
      phylogenetic: no

as previously introduced¹ (but not yet released).  As it was never
released, don't bother supporting the latter schema at all anymore.

Change first arose out of discussion started by @victorlin in a related
PR.²  Subsequent discussion with @tsibley³ led to dropping support for
the unreleased schema.

¹ <#461>
² <#462 (comment)>
³ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
tsibley added a commit that referenced this pull request Sep 24, 2025
List the workflow names on the same line as the pathogen name@version
when there are any workflows and emit nothing when there aren't.  This
fits more in the style of the rest of `nextstrain version` and is much
less cluttered (more information dense), particularly considering that
we, in short order too, expect ~all of our pathogens (and all their
future versions) to have the same 2–3 workflows listed (nextclade,
ingest, phylogenetic).

Prompted by feedback from @tsibley.¹

¹ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
@tsibley
Copy link
Contributor

tsibley commented Sep 24, 2025

Repushed with tweaks to the code and commit messages.

@tsibley
Copy link
Contributor

tsibley commented Sep 24, 2025

I'll merge once I get tests to pass…

joverlee521 and others added 3 commits September 24, 2025 15:19
…run` compatibility

Workflows are now registered under a top level "workflows" key in the
nextstrain-pathogen.yaml file, and each workflow has its own
"compatibility" declarations.  The only current "compatibility"
declaration remains "nextstrain run".

This allows for more meta information about each workflow to be
registered in one place not under the banner of compatibility.

The nextstrain-pathogen.yaml schema changes to a workflow-first
structure like:

  workflows:
    ingest:
      compatibility:
        nextstrain run: yes
    phylogenetic:
      compatibility:
        nextstrain run: no

instead of a compatibility-first structure like:

  compatibility:
    nextstrain run:
      ingest: yes
      phylogenetic: no

as previously introduced¹ (but not yet released).  As it was never
released, don't bother supporting the latter schema at all anymore.

Change first arose out of discussion started by @victorlin in a related
PR.²  Subsequent discussion with @tsibley³ led to dropping support for
the unreleased schema.

¹ <#461>
² <#462 (comment)>
³ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
List the workflow names on the same line as the pathogen name@version
when there are any workflows and emit nothing when there aren't.  This
fits more in the style of the rest of `nextstrain version` and is much
less cluttered (more information dense), particularly considering that
we, in short order too, expect ~all of our pathogens (and all their
future versions) to have the same 2–3 workflows listed (nextclade,
ingest, phylogenetic).

Prompted by feedback from @tsibley.¹

¹ <#472 (comment)>

Co-authored-by: Thomas Sibley <tsibley@fredhutch.org>
Helpful to ensure that their directories exist as expected to catch
registration vs. reality mismatches.
@tsibley tsibley force-pushed the workflow-compatibility branch from 8e98a95 to 20c99d4 Compare September 24, 2025 22:20
@tsibley tsibley merged commit 159faf0 into master Sep 25, 2025
56 checks passed
@tsibley tsibley deleted the workflow-compatibility branch September 25, 2025 16:09
joverlee521 added a commit to nextstrain/measles that referenced this pull request Sep 26, 2025
joverlee521 added a commit to nextstrain/zika that referenced this pull request Sep 26, 2025
joverlee521 added a commit to nextstrain/pathogen-repo-guide that referenced this pull request Oct 31, 2025
Workflow changes to support running the workflows with the 
`nextstrain run` command.

I specifically chose _not_ to declare compatibility for `nextstrain run` in 
the `nextstrain-pathogen.yaml` because the phylogenetic and nextclade workflows 
are incomplete in this guide. Once workflow specific compatibility is 
supported,¹ we can declare compatibility for the ingest workflow.

¹ <nextstrain/cli#472>
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

Successfully merging this pull request may close these issues.

3 participants