aiidateam / aiida-core

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

👌 Improve usage of `configure-rabbitmq` #6474

Closed mbercx closed 1 week ago

mbercx commented 2 weeks ago

Implements the suggestions in https://github.com/aiidateam/aiida-core/pull/6454#issuecomment-2155873817.

Example UX:

❯ brew services stop rabbitmq
Stopping `rabbitmq`... (might take a while)
==> Successfully stopped `rabbitmq` (label: homebrew.mxcl.rabbitmq)
❯ verdi presto
Report: Option `--use-postgres` not enabled: configuring the profile to use SQLite.
Report: RabbitMQ server not found: configuring the profile without a broker.
Report: Initialising the storage backend.
Report: Storage initialisation completed.
Success: Created new profile `presto-7`.
Success: Configured the localhost as a computer.
❯ verdi status
 ✔ version:     AiiDA v2.5.1.post0
 ✔ config:      /Users/mbercx/project/core/.aiida
 ✔ profile:     presto-7
 ✔ storage:     SqliteDosStorage[/Users/mbercx/project/core/.aiida/repository/sqlite_dos_9334f1ac2f1f42d2b51724491fdb9030]: open,
 ⏺ broker:      No broker defined for this profile: certain functionality not available.
 ⏺ daemon:      No broker defined for this profile: daemon is not available.
❯ brew services start rabbitmq
==> Successfully started `rabbitmq` (label: homebrew.mxcl.rabbitmq)
❯ verdi profile configure-rabbitmq
Success: Successfully connected to RabbitMQ server.
❯ verdi status
 ✔ version:     AiiDA v2.5.1.post0
 ✔ config:      /Users/mbercx/project/core/.aiida
 ✔ profile:     presto-7
 ✔ storage:     SqliteDosStorage[/Users/mbercx/project/core/.aiida/repository/sqlite_dos_9334f1ac2f1f42d2b51724491fdb9030]: open,
Warning: RabbitMQ v3.13.1 is not supported and will cause unexpected problems!
Warning: It can cause long-running workflows to crash and jobs to be submitted multiple times.
Warning: See https://github.com/aiidateam/aiida-core/wiki/RabbitMQ-version-to-use for details.
 ✔ broker:      RabbitMQ v3.13.1 @ amqp://guest:guest@127.0.0.1:5672?heartbeat=600
 ⏺ daemon:      The daemon is not running.
codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.76%. Comparing base (ef60b66) to head (a49e79a). Report is 49 commits behind head on main.

Files Patch % Lines
src/aiida/cmdline/commands/cmd_profile.py 91.67% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6474 +/- ## ========================================== + Coverage 77.51% 77.76% +0.26% ========================================== Files 560 561 +1 Lines 41444 41781 +337 ========================================== + Hits 32120 32487 +367 + Misses 9324 9294 -30 ```

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

mbercx commented 1 week ago

@sphuber I made the requested changes:

  1. The NON_INTERACTIVE option has been replaced with INTERACTIVE. This is the switch we discussed, but to me it was more natural to have "interactive" to avoid double negatives in the logic.
  2. The configure-rabbitmq command now checks if it can successfully connect to the broker, if not it will ask for confirmation to configure, unless -f/--force is specified. It's a bit conflicting with the --non-interactive option, I suppose, but ok.

Here is the usage:

❯ verdi presto 
Report: Option `--use-postgres` not enabled: configuring the profile to use SQLite.
Report: RabbitMQ server not found: configuring the profile without a broker.
Report: Initialising the storage backend.
Report: Storage initialisation completed.
Success: Created new profile `presto-9`.
Success: Configured the localhost as a computer.
❯ verdi profile configure-rabbitmq 
Warning: Unable to connect to RabbitMQ server with configuration: {'protocol': 'amqp', 'username': 'guest', 'password': 'guest', 'host': '127.0.0.1', 'port': 5672, 'virtual_host': ''}
Do you want to continue with the provided configuration? [y/N]: N
Aborted!
❯ brew services start rabbitmq
==> Successfully started `rabbitmq` (label: homebrew.mxcl.rabbitmq)
❯ verdi profile configure-rabbitmq
Success: Connected to RabbitMQ with the provided connection parameters
Success: RabbitMQ configuration for `presto-9` updated to: {'protocol': 'amqp', 'username': 'guest', 'password': 'guest', 'host': '127.0.0.1', 'port': 5672, 'virtual_host': ''}
❯ brew services stop rabbitmq
Stopping `rabbitmq`... (might take a while)
==> Successfully stopped `rabbitmq` (label: homebrew.mxcl.rabbitmq)
❯ verdi profile configure-rabbitmq
Warning: Unable to connect to RabbitMQ server with configuration: {'protocol': 'amqp', 'username': 'guest', 'password': 'guest', 'host': '127.0.0.1', 'port': 5672, 'virtual_host': ''}
Do you want to continue with the provided configuration? [y/N]: N
Aborted!
❯ verdi profile configure-rabbitmq -f
Warning: Unable to connect to RabbitMQ server with configuration: {'protocol': 'amqp', 'username': 'guest', 'password': 'guest', 'host': '127.0.0.1', 'port': 5672, 'virtual_host': ''}
Success: RabbitMQ configuration for `presto-9` updated to: {'protocol': 'amqp', 'username': 'guest', 'password': 'guest', 'host': '127.0.0.1', 'port': 5672, 'virtual_host': ''}

I was still considering making the final Success: a Report: instead.

mbercx commented 1 week ago

@sphuber will update the commits properly once we've agreed on the changes.

mbercx commented 1 week ago

@sphuber the NON_INTERACTIVE option has been reinstated, commit messages updated.

sphuber commented 1 week ago

@sphuber the NON_INTERACTIVE option has been reinstated, commit messages updated.

Fantastic, thanks a lot for going through that effort, much appreciated