Make koha-disable handle errors more gracefully. Also make it support more than one instance as parameter (there is a for loop, but a previous check prevents it to work).
Created attachment 17626 [details] [review] Bug 10104 - make koha-disable more robust koha-disable now: - checks for the existence of the instance before any actions on it. - checks if the instance is already disabled before touching anything (warns otherwise) - only reloads apache if needed - handles more than one instance name. - changed the docs to acknowledge the previous item. To test: - Apply the patch, build your package - Run koha-disable on - Non existent instance (try names that are prefix and suffix of a valid one too please) - Already disabled instance name. - Enabled instance name. It should work as expected and warn the user on the expected wrong cases. Regards To+ Sponsored-by: Universidad Nacional de Córdoba
Testing now On a disabled instance sudo ./debian/scripts/koha-disable persona Instance persona already disabled. On an unknown instance sudo ./debian/scripts/koha-disable pers Unknown instance pers. On an enabled instance sudo ./debian/scripts/koha-disable persona [....] Restarting web server: apache2 ok Looks all good to me, I confirm its disabled.
Created attachment 17775 [details] [review] Signed off patch
a small QA question, this patch breaks the default 'usage' message, when no arg is passed - is this intended? -[ "$#" = 1 ] || die "Usage: $0 instancename..." +[ "$#" > 1 ] || die "Usage: $0 instancename..." (this is the same issue as bug-10101)
Created attachment 17812 [details] [review] Bug 10104 - Followup: fix param check As noted by Mason on bug 10101, the check was not working for zero arguments. Sponsored-by: Universidad Nacional de Córdoba
sudo ./debian/scripts/koha-disable Disables Koha instances. Usage: ./debian/scripts/koha-disable instancename1 instancename2... Missing instance name...
Created attachment 17819 [details] [review] Follow up signed off
Created attachment 17837 [details] [review] Bug 10104 - make koha-disable more robust koha-disable now: - checks for the existence of the instance before any actions on it. - checks if the instance is already disabled before touching anything (warns otherwise) - only reloads apache if needed - handles more than one instance name. - changed the docs to acknowledge the previous item. To test: - Apply the patch, build your package - Run koha-disable on - Non existent instance (try names that are prefix and suffix of a valid one too please) - Already disabled instance name. - Enabled instance name. It should work as expected and warn the user on the expected wrong cases. Regards To+ Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Mason James <mtj@kohaaloha.com>
Created attachment 17838 [details] [review] Bug 10104 - Followup: fix param check As noted by Mason on bug 10101, the check was not working for zero arguments. Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Mason James <mtj@kohaaloha.com>
(In reply to comment #7) > Created attachment 17819 [details] [review] [review] > Follow up signed off Chris, i think you forgot to add a sign-off line to your uploaded patch
> > Sponsored-by: Universidad Nacional de Córdoba > Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> > Signed-off-by: Mason James <mtj@kohaaloha.com> passing QA on 2 patches
This patch has been pushed to master and 3.12.x.
Pushed to 3.10.x will be in 3.10.6