verdi setup: forward broker defaults to interactive mode
#4405
Conversation
The options for the message broker configuration do define defaults, however, the interactive clones for `verdi setup`, which are defined in `aiida.cmdline.params.options.commands.setup` override the default with the `contextual_default` which sets an empty default, unless it is taken from an existing profile. The result is that for new profiles, the broker options do not specify a default, even though for most usecases the defaults will be required.
Codecov Report
@@ Coverage Diff @@
## develop #4405 +/- ##
===========================================
+ Coverage 79.22% 79.23% +0.01%
===========================================
Files 475 475
Lines 34826 34827 +1
===========================================
+ Hits 27587 27590 +3
+ Misses 7239 7237 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @sphuber , I guess this is in response to the recent post to the AiiDA mailing list?
The changes look fine to me and I'm approving this, but you might still consider adding a test to check that new profiles have the right settings (i.e. a test that would catch the issue we saw on the mailing list).
Also, I found it not so easy to follow the slightly convoluted sentence structure in the commit message - here an alternative suggestion:
The `verdi setup` cli was overriding default values for the message broker
configuration with `None`, leading to a broken profile configuration.
This did not affect `verdi quicksetup`, where proper defaults were used.
Not really. I haven't seen any post with respect to this? Which do you mean?
It is not actually a problem with the created profiles, they are fine. It is just that when running
Maybe this has to do with the previous point, but this message is incorrect. There was no risk of broken profiles, just the prompt did not provide sensible defaults. |
|
I have added an extra sentence to the commit message to make clarify that this just concerns the prompt displaying a proper default. |
|
The mailing list: https://groups.google.com/g/aiidausers/c/mxW8NfRwVJM |
Thanks. I don't think this is related. This exception is known to happen before the new broker configuration of v1.4. It is being discussed in this issue. So far we haven't been able to fully pin point the problem nor find a solution. I will respond to the mailing list with a link to the issue and ask if he can add his example. |
…#4405) The options for the message broker configuration do define defaults, however, the interactive clones for `verdi setup`, which are defined in `aiida.cmdline.params.options.commands.setup` override the default with the `contextual_default` which sets an empty default, unless it is taken from an existing profile. The result is that for new profiles, the broker options do not specify a default, even though for most usecases the defaults will be required. After the changes of this commit, the prompt of `verdi setup` will provide a default for all broker parameters so most users will simply have to press enter each time.
Fixes #4404
The options for the message broker configuration do define defaults,
however, the interactive clones for
verdi setup, which are defined inaiida.cmdline.params.options.commands.setupoverride the default withthe
contextual_defaultwhich sets an empty default, unless it is takenfrom an existing profile. The result is that for new profiles, the
broker options do not specify a default, even though for most use cases
the defaults will be required.