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

Improved and expanded runtime documentation #303

Merged
merged 10 commits into from
Aug 17, 2023
Merged

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Aug 3, 2023

See commit messages.

Resolves #288.

Future PRs?

Testing

  • Preview looks good
  • Checks pass

…time

I'd like to make this distinction even clearer in the future, but it'll
have to wait for another doc push.  For now, at least provide a pointer
to anyone coming to this page wanting to update the runtime.
Trying to provide a more useful resource to point people to than a
smattering of scattered doc snippets.

There's still some places this could be better cross-referenced (e.g. in
--help output) and more that could be said about runtimes in general and
each runtime specifically… but that can all come in due time.  I needed
to find a stopping point for this work _somewhere_.   A big gain here is
that now there's clear places to attach additional documentation in the
future.

Like the recent switch from dynamically- to statically-generated rST for
command documentation, it may also sooner-than-later make sense to ditch
the "automodule" directive and do something static instead.

Resolves <#288>.
@tsibley tsibley requested a review from a team August 3, 2023 01:03
@joverlee521 joverlee521 self-requested a review August 3, 2023 18:18
Since we immediately convert the Docker/OCI image to SIF, the Docker/OCI
image layer caches end up being duplicative.  Not keeping them cached
means the cache can't be used to avoid downloading layers that haven't
changed in subsequent updates, but I think when choosing between
minimizing transfer or storage for our users, we should minimize
storage.

If we want to have our cake and eat it to (i.e. minimize both), we might
be able to change tack to newer Singularity features where (I think) OCI
images can be used directly without first converting to SIF.  Then, like
with our Docker runtime, the image we use and layer caches are one and
the same.  That's a larger change for a future time, though.
@tsibley
Copy link
Member Author

tsibley commented Aug 3, 2023

Pushed another couple smaller commits addressing #225.

…ary with ours

More internally consistent now that we have the glossary term, which in
turn links to our overview page about them for more details.
Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

This will be super helpful for users to learn more about the different runtimes 🌟

This is definitely out-of-scope of this PR, but as I was reading the new docs I kept wondering if we should list the non-Nextstrain tools that are available in the runtimes. It's definitely discoverable if people go digging in the Dockerfile or Conda recipe.yaml but might be helpful to list the commonly used tools such as tsv-utils, csvtk, and seqkit.


Switching runtimes or updating the version of a runtime may result in different
versions of Nextstrain components like Augur and Auspice as well as other
programs, and thus different behaviour.
Copy link
Contributor

Choose a reason for hiding this comment

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

My immediate questions after reading this part:

  1. How do I know which versions are available in my current runtime?
  2. How can I find a version of a runtime that includes the specific version of a Nextstrain program that I want?

For [1], I think it'd be helpful to move or recap the sentence about nextstrain version --verbose here.

Not sure others may raise [2] but wanted to put it out there.

Copy link
Member

Choose a reason for hiding this comment

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

I found myself asking [2] recently when Augur 22.2.0 had a bug. It took me some digging in the Docker image's build logs to figure out that nextstrain/base:build-20230720T001758Z was the last runtime version with an earlier version of Augur.

I don't think there's an easy solution here.

One solution would be to maintain an up-to-date resource describing the versions of Nextstrain programs in each runtime version. For example:

runtime version Augur Auspice Nextstrain CLI
conda nextstrain-base 20230815T151610Z 22.3.0 2.46.0 7.1.0
conda nextstrain-base 20230805T043657Z 22.2.0 2.46.0 7.1.0
docker nextstrain/base:build-20230814T140428Z 22.3.0 2.46.0 7.1.0
docker nextstrain/base:build-20230805T043648Z 22.2.0 2.46.0 7.1.0

This would be straightforward to use, but probably not straightforward to maintain.

Copy link
Member Author

@tsibley tsibley Aug 17, 2023

Choose a reason for hiding this comment

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

Pushed 8b90a0b to address 1. Addressing 2 seems useful, I agree, but I think to provide a useful answer besides "binary search it yourself" we'll need to produce static reports for each runtime version of the included software (ours and others). This would also address your broader comment above.

(Update: I wrote this without seeing Victor's comment due to a stale tab! I do think such a report, like his example or otherwise, would be sustainable to maintain automatically.)

nextstrain/cli/runner/conda.py Outdated Show resolved Hide resolved
Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

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

Looks great! Left one small non-blocking suggestion.

nextstrain/cli/runner/ambient.py Outdated Show resolved Hide resolved

Switching runtimes or updating the version of a runtime may result in different
versions of Nextstrain components like Augur and Auspice as well as other
programs, and thus different behaviour.
Copy link
Member

Choose a reason for hiding this comment

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

I found myself asking [2] recently when Augur 22.2.0 had a bug. It took me some digging in the Docker image's build logs to figure out that nextstrain/base:build-20230720T001758Z was the last runtime version with an earlier version of Augur.

I don't think there's an easy solution here.

One solution would be to maintain an up-to-date resource describing the versions of Nextstrain programs in each runtime version. For example:

runtime version Augur Auspice Nextstrain CLI
conda nextstrain-base 20230815T151610Z 22.3.0 2.46.0 7.1.0
conda nextstrain-base 20230805T043657Z 22.2.0 2.46.0 7.1.0
docker nextstrain/base:build-20230814T140428Z 22.3.0 2.46.0 7.1.0
docker nextstrain/base:build-20230805T043648Z 22.2.0 2.46.0 7.1.0

This would be straightforward to use, but probably not straightforward to maintain.

…ng/updating runtimes

A good suggestion by @joverlee521 in review.
à la how we do it for the docker-base repo and the Docker runtime.

A good suggestion by @joverlee521 in review.
…f our ambient runtime

The general installation page contains a good example of ambient setup.

A good suggestion by @victorlin in review.
@tsibley tsibley merged commit 3e495e4 into master Aug 17, 2023
34 checks passed
@tsibley tsibley deleted the trs/docs/runtimes branch August 17, 2023 19:22
@tsibley
Copy link
Member Author

tsibley commented Aug 17, 2023

Consider how this doc integrates (or doesn't) with the umbrella docs

For this, I think I will limit work for now to adding a FAQ entry in the umbrella docs and cross-linking into this CLI reference documentation.

tsibley added a commit to nextstrain/docs.nextstrain.org that referenced this pull request Aug 18, 2023
…the new runtimes documentation

This hopefully clarifies a term we use without really explaining it, and
at least provides a pointer to more details for those who want to know
more.

Related-to: <nextstrain/cli#303>
tsibley added a commit to nextstrain/docs.nextstrain.org that referenced this pull request Aug 18, 2023
…the new runtimes documentation

This hopefully clarifies a term we use without really explaining it, and
at least provides a pointer to more details for those who want to know
more.

Related-to: <nextstrain/cli#303>
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.

Improve runtime documentation
3 participants