aiidateam / aiida-core

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

More user friendly yes/no prompt in `verdi computer delete` #6422

Open agoscinski opened 4 months ago

agoscinski commented 4 months ago

Describe the problem

Looking at the yes/no prompt of verdi computer delete

$ verdi computer delete <COMPUTER>
Report: This computer has 0 associated nodes
Report: 0 Node(s) marked for deletion
Shall I continue? [yes/N]:

Would suggest to do [y/N] or [yes/NO]. I tend for the former as it is used in Debian-based system, also I saw this used in other verdi shell prompts.

Also I would accept y,yes, n, no in both cases as valid input. Here again, my intuition comes from using a Debian-based system.

My environment

Python 3.11.0, Ubuntu 22.04

$ verdi status
 ✔ version:     AiiDA v2.5.1.post0
 ✔ config:      /home/alexgo/code/aiida-core/.aiida
 ✔ profile:     presto-1
 ✔ storage:     SqliteDosStorage[/home/alexgo/code/aiida-core/.aiida/repository/sqlite_dos_8665b32a7f4d45d3b1448d7931fd503d]: open,
 ✔ broker:      RabbitMQ v3.8.14 @ amqp://guest:guest@127.0.0.1:5672?heartbeat=600
 ⏺ daemon:      The daemon is not running.
sphuber commented 4 months ago

This was discussed in the PR that changed it. As you can see, the original proposed string was SuRe which I opposed. I am personally also simply for [y/N] (the capitalizad N is just to show that that is the default I believe). We compromised and settled on yes/N as @khsrali @GeigerJ2 and @giovannipizzi thought it dangerous to keep just y.

GeigerJ2 commented 4 months ago

So the idea here was that deleting a computer doesn't only delete the isolated computer. To keep provenance, it also deletes all the connected nodes, so all processes of a user that were run on this computer. So it's an operation that should only be done with great care. Personally, I'd also be fine with [y/N] as that is usually the standard. The fear was that people just quickly pressing through things might accidentally run the operation somewhat unintentional, though, that should be captured with [y/N]. Don't really have a strong opinion on the topic, for me either is fine. Ideally, as always, I'd vouch for consistency.

giovannipizzi commented 4 months ago

I'm ok with the original suggestion and generally keeping a consistent style across verdi commands, as long as the default stays N

khsrali commented 4 months ago

We thought perhaps using a different response than the usual "y" would make sense to prevent accidental data loss. As @sphuber said, we compromised to "yes". But if @agoscinski thinks that is confusing regarding standardization, one could replace with "please type: 'confirm deletion' ", like when github asks you when deleting your repo. This way remain both safe and standard.