aiidateam / aiida-core

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

CLI: Remove the RabbitMQ options from `verdi profile setup` #6480

Closed sphuber closed 1 day ago

sphuber commented 1 week ago

For the vast majority of use cases, users will have a default setup for RabbitMQ and so the default configuration will be adequate and so they will not need the options in the command. On the flipside, showing the options by default can makes the command harder to use as users will take pause to think what value to pass.

Since there is the verdi profile configure-rabbitmq command now that allows to configure or reconfigure the RabbitMQ connection parameters for an existing profile, it is fine to remove these options from the profile setup. Advanced users that need to customize the connection parameters can resort to that separate command.

sphuber commented 1 week ago

Logic for setting up rabbitmq when user sets it to False seems a bit weird to me.

Thanks @agoscinski . I misunderstood your comment and thought you wanted to have the hint for verdi profile configure-rabbitmq to be displayed always. I actually think this makes sense, but also agree that the current wording seems misleading because if the user specifies --no-use-rabbitmq it seems like the command still tried it but failed.

Anyway I have refactored it in the vein of your suggestion being extra clear of the three different pathways. All branches still reference the configure-rabbitmq command, but in the case of --no-use-rabbitmq the command clearly states first that the profile is not configuring RMQ as a broker. I think it should be clear enough now.

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 92.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.76%. Comparing base (ef60b66) to head (835053e). Report is 53 commits behind head on main.

Files Patch % Lines
src/aiida/cmdline/commands/cmd_profile.py 90.91% 2 Missing :warning:
src/aiida/cmdline/commands/cmd_presto.py 94.12% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6480 +/- ## ========================================== + Coverage 77.51% 77.76% +0.26% ========================================== Files 560 561 +1 Lines 41444 41795 +351 ========================================== + Hits 32120 32499 +379 + Misses 9324 9296 -28 ```

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