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

How to get/set config options which are from a dict #561

Closed
surak opened this issue Jul 8, 2022 · 6 comments
Closed

How to get/set config options which are from a dict #561

surak opened this issue Jul 8, 2022 · 6 comments

Comments

@surak
Copy link
Contributor

surak commented Jul 8, 2022

When setting the system I can check for a multi-line config option, such as

$ shpc config get wrapper_scripts
wrapper_scripts                ordereddict([('enabled', False), ('docker', 'docker.sh'), ('podman', 'docker.sh'), ('singularity', 'singularity.sh'), ('templates', None)])

But how can I set one of them back with shpc config --global set? The documentation doesn't mention this case.
This is for automated installations where the default configuration file comes from a git clone and only the relevant changes are made.

Any reason why the wrapper is disabled by default, by the way? One clearly needs that for slurm...

@vsoch
Copy link
Member

vsoch commented Jul 8, 2022

Ah! So the normal way (which I agree we need to document better) is to add a colon to separate levels

$ shpc config get wrapper_scripts:enabled
wrapper_scripts:enabled        False

But there is also a bug in the final step here that writes "is unset" if "not value" and it needs to just check if the value is None (and not false!) Set also has a bug - it was written before we had the nested values.

@surak I'll have a PR with fixes for the above and better docs this evening! And I do think we could have the default be wrapper scripts - I was actually thinking this yesterday! @marcodelapierre and @muffato would that make sense to you as well?

@muffato
Copy link
Contributor

muffato commented Jul 8, 2022

Hi @vsoch . In my world, yes, the wrapper scripts should be default

@vsoch
Copy link
Member

vsoch commented Jul 8, 2022

okay here is a start! #562

I am likely to add a few more test cases tonight (e.g., derivates of true/True false/False null/Null/None/none and the like, but the basic updates should be there!

@vsoch
Copy link
Member

vsoch commented Jul 8, 2022

Scratch that - added them now! :)

@marcodelapierre
Copy link
Contributor

Yep, here at Pawsey we also use wrapper scripts as default in our deployments.

@vsoch
Copy link
Member

vsoch commented Jul 11, 2022

Fixed with #562 - will be released shortly.

@vsoch vsoch closed this as completed Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants