aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
411 stars 184 forks source link

CLI: Add RabbitMQ options to `verdi profile setup` #6453

Closed sphuber closed 3 weeks ago

sphuber commented 3 weeks ago

The verdi profile setup command was added to replace the deprecated command verdi setup. The new command dynamically generates a subcommand for each installed storage plugin.

However, the new command did not allow to configure the connection parameters for the RabbitMQ broker, unlike verdi setup. These common options are now added to the subcommands.

In addition, a new option is added --use-rabbitmq/--no-use-rabbitmq. This flag is on by default, to keep the old behavior of verdi setup. When toggled to --no-use-rabbitmq, the RabbitMQ configuration options are no longer required and are also not prompted for. The profile is then configured without a broker.

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.87%. Comparing base (ef60b66) to head (19cbf11). Report is 34 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6453 +/- ## ========================================== + Coverage 77.51% 77.87% +0.37% ========================================== Files 560 562 +2 Lines 41444 41808 +364 ========================================== + Hits 32120 32555 +435 + Misses 9324 9253 -71 ``` | [Flag](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6453/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [presto](https://app.codecov.io/gh/aiidateam/aiida-core/pull/6453/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | `73.19% <14.29%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sphuber commented 3 weeks ago

Thanks for the review @GeigerJ2

mbercx commented 3 weeks ago

Hi guys, again sorry for being a late party-pooper. My main question is if we want to add these RabbitMQ-specific options to all of the setup commands. Now that we have a command for reconfiguring rabbitmq, can't we just use the defaults and then point the user to this command in case they want to use others?

I think we already foresee that we want to try and add a different broker than RabbitMQ. Presumably the options for this broker might be different. Are we going to add all the options for that broker here as well? Should we then also rename the rabbitmq options? (which are now all --broker-X).

I would stick with just adding --use-rabbitmq / --no-use-rabbitmq, use the defaults for the configuration, add a check to see if it worked and point them to the configure-rabbitmq command if it failed.