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

Should the use_login_shell property of the AuthInfo also be considered in the submission script shebang #5895

Closed
sphuber opened this issue Feb 16, 2023 · 7 comments · Fixed by #5905 or #5950

Comments

@sphuber
Copy link
Contributor

sphuber commented Feb 16, 2023

In PR #4271 an option was added use_login_shell to configure how commands are executed on a computer through the transport. If the option is switched to True all bash commands executed over the transport will use the -l flag to force using a login shell. However, this is not used for the submission script itself. The default shebang (which is configurable on the Computer) is #/bin/bash, but this never receives the -l flag, even if use_login_shell is set to True.

For certain use cases, the submission script is required to use a login shell. For example, when the submission script needs to load a conda environment, this will not work with a non-interactive shell.

So the question is: should the use_login_shell also impact the shebang of the submission script?

@sphuber
Copy link
Contributor Author

sphuber commented Feb 23, 2023

The use-case is already possible with currently available functionality. The Computer simply should set the shebang property to #!/bin/bash -l. This is a preferable solution for now because it is not quite clear that using the use_login_shell property of the AuthInfo to also affect the submission script shebang is fully backwards-compatible for all use cases.

@chrisjsewell
Copy link
Member

Hey, I would suggest this is not necessarily the best approach for conda, and that #5905 may need amending.

I note, there is unfortunately currently not a de facto way to activate in a script.
However, there are plenty of reasons why you may not want to start using a login script to run your calculations, and the generally accepted approach is:

eval "$(conda shell.bash hook)"
conda activate myenv

See conda/conda#7980 and also https://docs.conda.io/projects/conda/en/latest/dev-guide/deep-dives/activation.html.
This is also what is now used in quantum mobile, with no problems

cc @unkcpz @giovannipizzi @ltalirz

@chrisjsewell chrisjsewell reopened this Mar 29, 2023
@ltalirz
Copy link
Member

ltalirz commented Mar 29, 2023

You are right that one can use conda without requiring a login shell (as you describe).

On the other hand, if a user wants the convenience of being able to expect the same environment in AiiDA scripts as when they connect to the cluster interactively, I guess the use_login_shell option provides that functionality.

Is there an argument why AiiDA job scripts should work differently here with respect to other aspects (like ssh transport)?

@chrisjsewell
Copy link
Member

Is there an argument why AiiDA job scripts should work differently here with respect to other aspects (like ssh transport)?

Ah well this, I feel, is a different discussion, i.e. whether use_login_shell should be set by default to True (for any run script). Perhaps this is correct, I don't know off-hand

But, I am simply suggesting that the assertion that "conda environments will not work with a non-interactive shell" is incorrect, and it is not necessary to deviate from the current (non-login) default for run scripts.

@sphuber
Copy link
Contributor Author

sphuber commented Mar 30, 2023

But, I am simply suggesting that the assertion that "conda environments will not work with a non-interactive shell" is incorrect, and it is not necessary to deviate from the current (non-login) default for run scripts.

Would the solution be to update the docs that I added in #5905 to also mention the approach you mentioned. I have been using the login script solution in production without issues, so clearly it can be a solution in some cases. Since you say there is no de facto standard, it would seem ok to simply mention the two options. If you can mention a concrete reason why one would not want to use a login script, that could help users decide which one to use.

@chrisjsewell
Copy link
Member

Would the solution be to update the docs that I added in #5905 to also mention the approach you mentioned

Yep exactly cheers 👍 and maybe even add the links I gave

If you can mention a concrete reason why one would not want to use a login script, that could help users decide which one to use.

As already mentioned, this goes more to a general discussion of when to use login, that isn't linked per se directly to conda

@sphuber
Copy link
Contributor Author

sphuber commented Mar 30, 2023

Fixed in #5950

@sphuber sphuber closed this as completed Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment