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

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

Open
victorlin opened this issue Dec 7, 2023 · 4 comments
Open

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

victorlin opened this issue Dec 7, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@victorlin
Copy link
Member

victorlin commented Dec 7, 2023

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:

  • nextstrain view output indicates that it works and Auspice is being served at http://127.0.0.1:4000, however that URL cannot be accessed on the host computer.

  • nextstrain update conda returns output unintended for the eyes of our users:

    Checking for newer versions of Nextstrain CLI…
    
    nextstrain-cli is up to date!
    
    Updating conda runtime…
    Unsupported system/machine: Linux/aarch64
    
  • nextstrain update docker fails:

    Checking for newer versions of Nextstrain CLI…
    
    nextstrain-cli is up to date!
    
    Updating docker runtime…
    Updating Docker image from nextstrain/base to nextstrain/base:build-20231130T175113Z…
    
    
    Updating failed!
    

In nextstrain shell --conda:

  • nextstrain view works.

  • nextstrain update conda works.

  • nextstrain update docker fails:

    Checking for newer versions of Nextstrain CLI…
    
    nextstrain-cli is up to date!
    
    Updating docker runtime…
    Updating Docker image from nextstrain/base to nextstrain/base:build-20231130T175113Z…
    
    
    Updating failed!
    

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.
@victorlin victorlin added the bug Something isn't working label Dec 7, 2023
@corneliusroemer
Copy link
Member

corneliusroemer commented Dec 14, 2023

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: nextstrain/docker-base@68ce754

@tsibley
Copy link
Member

tsibley commented Jan 10, 2024

+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
Copy link
Member

tsibley commented Jan 25, 2024

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants