Shouldn't a warning result in a return code ($?) higher then zero?

[gerardjp]

Hi All,

I'm working on some wrapper software and noticed a wrong return code on the commandline:

root@srvyy:~# wp --allow-root --path=/var/www/www.test.nl plugin deactivate p3-profile
Warning: The 'p3-profile' plugin could not be found.
(nb: the plugin name is actually p3-profiler)
root@srvyy:~# echo $?
0
root@srvyy:~# ls /not-here
ls: cannot access /not-here: No such file or directory
root@srvyy:~# echo $?
2
root@srvyy:~#

Shouldn't it produce an errorlevel higher then zero or am I missing something?

Thanx a lot.

Kind regards,

Gerard.

[danielbachhuber]

Related #1415

Same consideration applies though: WP-CLI supports deactivating multiple plugins at once. Interested to hear your opinions on what WP-CLI should do.

[gerardjp]

Hi Daniel,

Thanx for your reply. After going through my memory archive :) ... Hereby my feedback. Hope it helps.

Rough pointers

• Low fruit: Every action that has either succes or fail can return 0 / 1
• Normally (seen in C code) you would choose a bitwise operator (going from 0 to 255).
• Good practice, and possibly obvious, the higher the return code the more severe the issue that was caught.
  Another option; Choose subdividing your 0-255 range and leave some room in between for future levels.
• Ranges:
  • wp-cli issues 1-25
  • wordpress install 26-50
  • Db connectivity 51-75
  • Misc 76-100
  • plugin ranged update 101-150

There's probably never 25 different DB issues, but you get the idea.

More on the actual level (and ranges). It's mainly to combine errors, so when having a funky WP installation that results in an error 27 and you find a certain db connect issue e.g. 56 at the same time. The return level should be 83. Hence the bitwise operator

Might be to elaborate, but I'm writing a wrapper between saltstack and wp-cli .. so I wouldn't mind 😜

Update all

Usually the return codes represent distinct levels and not an approximation. So ... As far as the plugins and especially the "update all" goes. Consider changing “all" into "all (updates) available” even it it’s just by definition and not actually renaming the the parameter or sub command. Therewith the more updates that failed the higher the return code. Possibly choose a reserved range to your liking e.g. between 100 and 150), and count the failed updates.

So when "update all" run on a WP instance that has 25 plugins of which 12 avail updates. return 100 when all goes well (well zero then actually) and 1 higher for all failed. You should of course mention the amount of failed plugin updates to the user and not the error level visibly, but this way the distinction can be properly caught at least.

Oh, and yield breakage number till all updates are executed, and then return the error. .. I think is good.

I would like a 'whiteboard session' whether it's good or not to relate the amount of succeeded updates to a distinct error level, but it's a start 😄

This is the first I can come up with. I hope it helps.

Kind regards,

Gerard.

[scribu]

I think a good place to look for inspiration would be package managers, like apt-get, which also support installing multiple packages in one go and have to report failed installs somehow.

[scribu]

Note that the most reliable way to check the results is by running wp plugin list afterwards to see what actually got installed.

[gerardjp]

@scribu: Good point on the apt-get! 👍

On the review afterwards ... I do that with Salt commands as well when 'apt updating' my servers, however that does mean 'human interpretation' of the output.

Using the 'wp plugin list' command after a (distributed and thus unattended) update implies saving state somewhere. So the before and after picture can be compared. Not an issues as such but something to keep in mind while designing the setup. At least in my case 😉

Good'day

[bdruth]

Related to this - even in the case when doing a wp plugin install of only a single plugin, when that plugin install fails (couldn't download the URL, find the plugin file, whatever) - the return code is still zero. When setting up automation via puppet/Chef, this presents a problem as it makes the success/failure indeterminate.

[danielbachhuber]

Contradiction we need to solve:

• Use a friendly error message when option is same value #1551 uses "Success" when you pass the same value.
• More consistent behavior for plugin (de)activation #1578 uses "Warning" when you try to activate an already active plugin.

[gerardjp]

Working on a Wordpress plugin that enables some new wp-cli commands .. I just noticed in the API docs:

WP_CLI::error( $msg ) - prints an error on STDERR and exits with code 1

(ref: https://github.com/wp-cli/wp-cli/wiki/API)

This suggests an actual return code ... Is the documentation erroneous or did I mis somehting 😃

Ta!

[fabiokr]

How about a --fail option that would force failure?

[ypid]

wp plugin uninstall $active_plugin

should return something else than 0 …

[danielbachhuber]

For those subscribed to this thread, I've posted a RFC on changing the behavior #3577

[danielbachhuber]

This is being addressed by #3577