In 19.11.05 (and certainly other versions affected by Bug 17532), koha-foreach will exit early if 1 command for 1 instance has a non-zero status. It seems to me that we should warn on a failure, but we should continue the for loop, because the failure could be instance (rather than system) specific. In my case, /usr/share/koha/bin/cronjobs/overdue_notices.pl isn't running for most instances, because one of the instances starting with A is encountering this error: "No branches with active overduerules at /usr/share/koha/bin/cronjobs/overdue_notices.pl line 347". While I'll address that individually, it's an unfortunate consequence of Bug 17532 to ditch out early. I'm thinking the easiest solution would be to remove "set -e" from koha-foreach, and warn on individual failures.
I suppose there might be times where you want a koha-foreach to die if any command fails, but perhaps we should add a flag to koha-foreach for that scenario...
Created attachment 105453 [details] [review] Bug 25634: Warn if koha-shell returns non-zero in koha-foreach By putting koha-shell in an if statement, "set -e" will no longer cause the entire koha-foreach program to exit, if koha-shell returns a non-zero status. If a non-zero status is returned, we warn on it, and the caller of koha-foreach can interpret that command visually. To Test: 1) Write a shell script that says "Hello" and then exits with a 1 status 2) Run koha-foreach against that shell script with multiple instances available to koha-foreach 3) Before the patch, koha-foreach will die after the first "Hello" 4) After the patch, koha-foreach will continue, through all the instances, and report which instances returned non-zero statuses by which command.
(In reply to David Cook from comment #0) > I'm thinking the easiest solution would be to remove "set -e" from > koha-foreach, and warn on individual failures. Actually, the easiest solution was to wrap koha-shell in an if statement ad just warn on what instance and command return a non-zero status. "set -e" won't exit if the non-zero status is returned by a command wrapped by an if statement.
I agree, should not we display the error code?
(In reply to Jonathan Druart from comment #4) > I agree, should not we display the error code? Mmm good point. I'll add an alternative patch.
Created attachment 105478 [details] [review] Bug 25634: [Alternate] Warn if koha-shell returns non-zero in koha-foreach By putting koha-shell in an if statement, "set -e" will no longer cause the entire koha-foreach program to exit, if koha-shell returns a non-zero status. If a non-zero status is returned, we warn on it, and the caller of koha-foreach can interpret that command visually. To Test: 1) Write a shell script that says "Hello" and then exits with a 1 status 2) Run koha-foreach against that shell script with multiple instances available to koha-foreach 3) Before the patch, koha-foreach will die after the first "Hello" 4) After the patch, koha-foreach will continue, through all the instances, and report which instances returned non-zero statuses by which command.
Created attachment 105636 [details] [review] Bug 25634: [Alternate] Warn if koha-shell returns non-zero in koha-foreach By putting koha-shell in an if statement, "set -e" will no longer cause the entire koha-foreach program to exit, if koha-shell returns a non-zero status. If a non-zero status is returned, we warn on it, and the caller of koha-foreach can interpret that command visually. To Test: 1) Write a shell script that says "Hello" and then exits with a 1 status 2) Run koha-foreach against that shell script with multiple instances available to koha-foreach 3) Before the patch, koha-foreach will die after the first "Hello" 4) After the patch, koha-foreach will continue, through all the instances, and report which instances returned non-zero statuses by which command. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 105648 [details] [review] Bug 25634: [Alternate] Warn if koha-shell returns non-zero in koha-foreach By putting koha-shell in an if statement, "set -e" will no longer cause the entire koha-foreach program to exit, if koha-shell returns a non-zero status. If a non-zero status is returned, we warn on it, and the caller of koha-foreach can interpret that command visually. To Test: 1) Write a shell script that says "Hello" and then exits with a 1 status 2) Run koha-foreach against that shell script with multiple instances available to koha-foreach 3) Before the patch, koha-foreach will die after the first "Hello" 4) After the patch, koha-foreach will continue, through all the instances, and report which instances returned non-zero statuses by which command. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected, thanks. Passing QA
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.01
backported to 19.11.x for 19.11.07
Backported to 19.05.x branch for 19.05.12