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

Improvements for non-docker (native) usage #164

Open
jameshadfield opened this issue Mar 31, 2022 · 2 comments
Open

Improvements for non-docker (native) usage #164

jameshadfield opened this issue Mar 31, 2022 · 2 comments

Comments

@jameshadfield
Copy link
Member

Context: This issue comes from this PR comment thread

When installing nextstrain-cli via our install instructions in a native environment, step 5:

Confirm that the installation worked, nextstrain check-setup --set-default

does more than simply confirm it works, it's critical to setting up the CLI itself if you are a native user. Without this, the default runner is docker and so executing a build/view command results in the following error:

Error executing into ['docker', 'run', ...]: [Errno 2] No such file or directory: b'/sbin/docker'

Improvements

Option 1
We shouldn't attempt to run in docker when it is not available. We could either detect that docker is not available (which we do during nextstrain check-setup) and display help instead, or catch the error and display help. Help could be along the lines of:

We are attempting to run in the docker runtime environment as that is {the default | what your config specifies} however this is not available. You can specify a runtime explicitly, such as nextstrain {cmd} --native {args} or set the default via nextstrain check-setup --set-default.

Option 2
If there is only one runtime supported (as seen in the output of nextstrain check-setup) and no runtime is specified in ~/.nextstrain/config, then running nextstrain {cmd} should use the only supported runtime. This would be my interpretation of "It Just Works".

Defaults aren't sandboxed

Noticed while exploring this, is that the default runner is stored in ~/.nextstrain/config (on MacOS at least). We encourage users to install the CLI in conda environments, however the config lives out of the environment. This is broadly consistent with other tools (e.g. git's config is in ~/.gitconfig) and so is probably considered a feature not a bug, but it was something I noticed for the first time here.

@victorlin
Copy link
Member

This is my proposed solution, I think some variant of option 1:

Don't use any default runtime if the core.runner part is missing from ~/.nextstrain/config (or the file itself is missing, which I think is the case for first-time users).
For any subcommand that requires a runtime (build, view), print out a nice message when no runtime has been set e.g.

ERROR: Nextstrain runtime not set. Please run:

    nextstrain check-setup --set-default

This forces naive users to be familiar with the different runtime options.

@tsibley
Copy link
Member

tsibley commented Apr 8, 2022

@victorlin That change would break several existing usages in our CI and unknown others out in the wild. I'm disinclined to make it as I'm not sure what benefit it provides over just improving the error message when the runtime isn't available.

This forces naive users to be familiar with the different runtime options.

While I absolutely think we should have good, clear docs and explanations for the different runtime options, I don't think it's actually a good goal to force users to be familiar with them, esp. at first. A good goal in my mind (which we've been making progress towards) is to make the runtimes as interchangeable as possible.

Either of @jameshadfield's options basically sound reasonable to me, although I think I have a preference for option 1 for now with graduation to option 2 later once some improvements to how check-setup works make that more reasonable to quietly auto-run on first nextstrain build. That is, I think good UX for option 2 will be harder to implement right now with the current codebase and option 1 would be much easier, so do option 1 now and do option 2 later after making some more foundational improvements.

@victorlin victorlin moved this from New to Backlog in Nextstrain planning (archived) Apr 27, 2022
@tsibley tsibley mentioned this issue May 20, 2022
2 tasks
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
Status: Backlog
Development

No branches or pull requests

3 participants