-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
verdi plugin list
: Show which exit codes invalidate cache
#5710
verdi plugin list
: Show which exit codes invalidate cache
#5710
Conversation
cc4f2a7
to
6cbb7b1
Compare
d47b1d9
to
71446ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks @sphuber !
Funny, I was preparing an alternative layout to your original one with the [x], and it was pretty much exactly the changes you made by yourself (color, remove the colon) ;-)
The only further change I would suggest is to move the explanation of the colors to the bottom, below the list.
P.S. While thinking about how to present this list, I started wondering whether our default (valid cache = true) is the right choice.
As a plugin developer, when everything goes well, you don't need to return an exit code and it will pick 0 by default. But when something goes wrong, I think by default you want the result to be invalid for the cache, unless you explicitly say that "something went wrong but the result is still usable".
Anyhow, it's a bit late to change that now.
P.P.S 0 is not part of the list but is certainly a valid exit code.
71446ad
to
6eac855
Compare
Yeah, you may be right here.
I'm afraid so 😅. You could open an issue and we could try to think if we can provide a deprecation pathway to change the default by v3.0. It might be possible to do this.
Done! The only thing I noticed when doing this (actually when writing the commit message) is that this way of styling (i.e. without explicit symbol) will be problematic for terminals that don't support bold or colored output. There the user won't see any difference. Since we were already relying on bold output to have particular meaning (for inputs and outputs being required or not), I decided that we can keep it for now. But this could be a problem at some point. Not sure how common it is for terminals to not support bold colored text. |
For `Process` entry points, the `verdi plugin list` command will show a formatted rendering of the process specification. Here we mark exit codes that invalidate the cache in bold red. For example, the `ArithmeticAddCalculation` will now display as: $ verdi plugin list aiida.calculations core.arithmetic.add ... Exit codes: 0 The process finished successfully. 1 The process has failed with an unspecified error. 2 The process failed with legacy failure mode. 11 The process did not register a required output. 100 The process did not have the required `retrieved` output. 110 The job ran out of memory. 120 The job ran out of walltime. 310 The output file could not be read. 320 The output file contains invalid output. 410 The sum of the operands is a negative number. Exit codes that invalidate the cache are marked in bold red.
6eac855
to
3cffd22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this addition @sphuber !
very useful
Fixes #5707
For
Process
entry points, theverdi plugin list
command will show aformatted rendering of the process specification. Here we mark exit
codes that invalidate the cache by printing in bold red. For example,
the
ArithmeticAddCalculation
will now display as: