nextstrain / cli

The Nextstrain command-line interface (CLI)—a program called nextstrain—which aims to provide a consistent way to run and visualize pathogen builds and access Nextstrain components like Augur and Auspice across computing environments such as Docker, Conda, and AWS Batch.
https://docs.nextstrain.org/projects/cli/
MIT License
27 stars 20 forks source link

CLI-ception: Some commands do not work in managed runtime shells #336

Open victorlin opened 7 months ago

victorlin commented 7 months ago

Nextstrain CLI is currently provided in our managed runtimes, but this "CLI-ception" comes with some odd behavior.

It's only noticeable in a few commands related to runtime management (setup, update) and implementation details (view). I haven't tested, but most other commands such as build, deploy, remote should work just fine.

In nextstrain shell --docker:

In nextstrain shell --conda:

Potential solutions

  1. ⛔️ Remove Nextstrain CLI from managed runtimes. This forces users to only use nextstrain shell for interactive usage of augur, auspice, etc. but also prevents things like running nextstrain deploy from within a build.
  2. ⛔️ Detect presence within a shell and disable Nextstrain CLI entirely. Keep it accessible when using the runtime through build.
  3. Make CLI work within a managed runtime shell:
    1. For view within a Docker shell, expose the Auspice server to the host machine.
    2. For runtime management commands, detect presence within a managed runtime and disable those commands, since it does not make sense to use those in that context.
corneliusroemer commented 6 months ago

Thanks for writing this up! I don't think we should remove the CLI because some subcommands don't make sense or don't work, so we shouldn't pursue option 1.

Option 3 sounds best to me, just set an environment variable when we use shell and disable certain nextstrain commands based on the presence of that variable.

Going through blame of docker-base reveals that we added the CLI in Feb 2020 in this commit: https://github.com/nextstrain/docker-base/commit/68ce75492f996a980acfd4d73d4baed3f92a1017

tsibley commented 5 months ago

+1 for writing this up. These restrictions were all known to me—nextstrain remote and related commands are all that's really intended/expected to work inside a runtime—but obviously we could do better at making the restrictions more explicit.

I'd also favor option 3, with the exception that I don't think we need to make nextstrain view work within nextstrain shell --docker (at least initially). There are complications for doing so. (Also, recall that Docker and Conda are not our only runtimes.)

I don't think we need to prioritize this right away, FWIW.

tsibley commented 5 months ago

with the exception that I don't think we need to make nextstrain view work within nextstrain shell --docker (at least initially). There are complications for doing so.

otoh… https://discussion.nextstrain.org/t/unable-to-view-zika-tutorial/1533/8

victorlin commented 5 months ago

Maybe the easiest thing to do is to is detect presence within a shell and disable view and runtime management subcommands with a useful message. Example:

(Nextstrain shell) $ nextstrain view .
ERROR: nextstrain view cannot be run within a Nextstrain shell. Exit the shell and re-run the command.