Bug 10104 - koha-disable error handling
Summary: koha-disable error handling
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Chris Cormack
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 4876
  Show dependency treegraph
 
Reported: 2013-04-23 16:18 UTC by Tomás Cohen Arazi
Modified: 2013-12-05 20:05 UTC (History)
3 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10104 - make koha-disable more robust (4.02 KB, patch)
2013-04-23 16:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Signed off patch (4.07 KB, patch)
2013-04-28 19:45 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10104 - Followup: fix param check (1.34 KB, patch)
2013-04-29 18:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Follow up signed off (1.34 KB, patch)
2013-04-29 20:06 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10104 - make koha-disable more robust (4.16 KB, patch)
2013-04-30 07:17 UTC, Mason James
Details | Diff | Splinter Review
Bug 10104 - Followup: fix param check (1.42 KB, patch)
2013-04-30 07:19 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2013-04-23 16:18:12 UTC
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).
Comment 1 Tomás Cohen Arazi 2013-04-23 16:34:50 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-04-28 19:44:16 UTC
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.
Comment 3 Chris Cormack 2013-04-28 19:45:05 UTC Comment hidden (obsolete)
Comment 4 Mason James 2013-04-29 03:40:48 UTC
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)
Comment 5 Tomás Cohen Arazi 2013-04-29 18:23:10 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2013-04-29 20:01:22 UTC
 sudo ./debian/scripts/koha-disable                   
Disables Koha instances.

Usage: ./debian/scripts/koha-disable instancename1 instancename2...

Missing instance name...
Comment 7 Chris Cormack 2013-04-29 20:06:50 UTC Comment hidden (obsolete)
Comment 8 Mason James 2013-04-30 07:17:42 UTC
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>
Comment 9 Mason James 2013-04-30 07:19:11 UTC
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>
Comment 10 Mason James 2013-04-30 07:22:24 UTC
(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
Comment 11 Mason James 2013-04-30 07:23:57 UTC
> 
> 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
Comment 12 Jared Camins-Esakov 2013-05-02 02:33:51 UTC
This patch has been pushed to master and 3.12.x.
Comment 13 Chris Cormack 2013-05-09 09:18:28 UTC
Pushed to 3.10.x will be in 3.10.6