TUM-DAML / seml

SEML: Slurm Experiment Management Library
Other
165 stars 29 forks source link

Adds CLI option to confirm changes #62

Closed n-gao closed 2 years ago

n-gao commented 2 years ago

Reference issue

Right now some commands may require confirmation and the user has to be prompted via the shell.

What does this implement/fix?

This change enables one to confirm all changes in advance by supplying -y, e.g.,

seml <collection> cancel -y

This works for cancel, reload-sources, delete, reset, clean-db.

Additional information

Additionally the hard-coded thresholds of 10 for deleting and resetting have been replaced by user adjustable settings.

gasteigerjo commented 2 years ago

Do we want separate thresholds for those 3 commands? I think one joint threshold would be cleaner. Something like CONFIRM_DANGER_THRESHOLD.

n-gao commented 2 years ago

I thought about that too. I decided on three different settings because it is a bit more flexible for a relatively small additional amount of complexity. For instance, I personally keep the cancel confirmation at 1 with the rest at 5.

danielzuegner commented 2 years ago

Do you know how this will work with the chaining? Will we only have to supply -y once for the whole chain or multiple times?

danielzuegner commented 2 years ago

The "old" way of deleting / cancelling experiments prints something like this:

seml chain_commands delete -s               
Deleting 72 configurations from database collection. Are you sure? (y/n)

With the -y option it does not print the Deleting 72 configurations from database collection. part anymore (e.g., for cancelling it does not print any output anymore). Can you change this so that -y still prints to the console how many experiments are being cancelled / deleted?

n-gao commented 2 years ago

Do you know how this will work with the chaining? Will we only have to supply -y once for the whole chain or multiple times?

You should add -y for each subcommand. Imho that's intended. We could also move the -y to the main parser and only supply a single one.

The "old" way of deleting / cancelling experiments prints something like this:

seml chain_commands delete -s               
Deleting 72 configurations from database collection. Are you sure? (y/n)

With the -y option it does not print the Deleting 72 configurations from database collection. part anymore (e.g., for cancelling it does not print any output anymore). Can you change this so that -y still prints to the console how many experiments are being cancelled / deleted?

Will do.

gasteigerjo commented 2 years ago

I think it's better like it is now. If you run seml coll cancel -b 8 delete -b 8 -y it will now still ask once, but not the 2nd time.

It general, it might be good to have some overarching options that apply to all chained commands (such as -id, -b, -f). But that seems like quite a bit of additional effort