aiida-core
aiida-core copied to clipboard
CLI: Add the subcommand `verdi computer export`
Enables the export of the configuration and setup of a computer to yaml file similar to verdi code export. Since the setup and configuration are two seperate steps, one has to specify with config or setup which of the two should be exported.
There was the option to implement the commands verdi computer configure export and verdi computer setup export but since verdi computer setup does not take any additional required arguments it would be less intuitive for the user to have an optional additional argument that changes the command type. There was also the option to export both files in one command, but that seems to be less consistent with the rest of the CLI. The word config is used in verdi computer export config since in this case we are specifying the configuration, but it might be a bit confusing with verdi config (which I think should be called verdi configure for consistency). If anyone has strong opinions I can also rename it to verdi computer export configuration or verdi computer export configure as the latter seems to be used in other projects like aiida-core-registry
~~I am adding tests now, but already open a draft for comments~~
Relevant issues:
- https://github.com/aiidateam/aiida-core/issues/3521
- https://github.com/aiidateam/aiida-project/issues/3
Very nice work, thanks @agoscinski! For me, the naming is fine. I just dogfooded it a bit by running
verdi computer export --help, and exporting and re-importing mydaint-gpuandlocalhost(fromverdi presto) computers. Everything worked well, I just noted a few minor things (see code comments). In addition, when exportinglocalhost, theconfigYAML only containssafe_interval: 0, so using the interactiveverdi computer configstill asks forUse login shell when executing command. Is it possible to also include that in the exported YAML, so that all possible options are contained there? This was the case fordaint-gpu, where without running-n/--non-interactiveduring import I didn't get prompted for anything as everything was contained in the YAML file. If not, it's also fine as it's just one option and using-nsets the default.
This sound like verdi presto should set the use_login_shell configuration since it is a required argument in the regular verdi computer configure. Is it okay if I change it on that side in the code? What do you think @GeigerJ2 ?
This sound like
verdi prestoshould set theuse_login_shellconfiguration since it is a required argument in the regularverdi computer configure. Is it okay if I change it on that side in the code? What do you think @GeigerJ2 ?
Forwarding that to @sphuber as verdi presto is his baby :D I'm not sure if I remember correctly, but I think Gio once mentioned to me that use_login_shell could make things slower (I might be wrong, though, and it could have been another option). If so, we should then set a reasonable default there.
I squashed all changes from last review into the first commit, so all new commits are from the second round. The incomplete config @GeigerJ2 mentioned when using verdi presto is still an issue, but I don't know where I should solve it. The cmdline submodule is definitely the wrong place to impose defaults.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 77.69%. Comparing base (
ef60b66) to head (47cf99c). Report is 112 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #6389 +/- ##
==========================================
+ Coverage 77.51% 77.69% +0.18%
==========================================
Files 560 562 +2
Lines 41444 41699 +255
==========================================
+ Hits 32120 32392 +272
+ Misses 9324 9307 -17
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks a lot @agoscinski