aiidateam / aiida-core

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

CLI: Different error handling results in differently colored error messages #6416

Open agoscinski opened 4 months ago

agoscinski commented 4 months ago

This is the script

$ verdi process watch 1
Usage: verdi process watch [OPTIONS] [PROCESSES]...
Try 'verdi process watch --help' for help.

Error: Invalid value for '[PROCESSES]...': no ProcessNode found with ID<1>: No result was found
$ verdi process watch 4
Report: watching for broadcasted messages, press CTRL+C to stop...
Error: Process<4> is already terminated

and this is how it looks like in the terminal image Notice that the Error is colored in the second case and in the first not. The second occurrence is caused by aiida.cmdline.commands.utils.echo.echo_error, the first one I don't know. I noticed this mainly because I wanted to track the first error message, but could not find it in the code. Also checked plumpy, but grep did not give me anything and debugging through click is a bit cumbersome.

I think the issue that the coloring is inconsistent is not a high-priority issue, but I was mainly interested where the first error message origins from. Is it due to the decorator arguments that passes it to orm.querybuilder somehow? I could not reconstruct the whole logic flow here, but I saw that in querybuilder there is something that could output part of the error message.

My environment

Python version 3.11.0, Ubuntu 22.04

$ verdi status
 ✔ version:     AiiDA v2.4.3 # but I it is also in v2.5.1
 ✔ config:      /home/alexgo/code/aiida-restapi/.aiida
 ✔ profile:     alexgo
 ✔ storage:     Storage for 'alexgo' [open] @ postgresql://aiida_qs_alexgo_b19baf4f5e5f017f3a035308a01b19e4:***@localhost:5430/alexgo_alexgo_b19baf4f5e5f017f3a035308a01b19e4 / DiskObjectStoreRepository: adaf7710d9684ea09d1b191e3b17b8bd | /home/alexgo/code/aiida-restapi/.aiida/repository/alexgo/container
 ✔ rabbitmq:    Connected to RabbitMQ v3.8.14 as amqp://guest:guest@127.0.0.1:5672?heartbeat=600
 ✔ daemon:      Daemon is running with PID 2623893
sphuber commented 4 months ago

The first error is rendered by click itself whenever an exception (of a certain type, defined by click) is raised during the parsing of command line parameters. I personally don't think we should start messing with this just so the coloring can be more consistent tbh

agoscinski commented 4 months ago

I agree with you that this probably causes more work than it is worth it. I saw this package that colors the help messages of click https://github.com/click-contrib/click-help-colors/tree/master so it seems like it is not just one argument in click that can be set to change the error message style. I put priority on nice-to-have and remove that this is a bug.