Bug 10101 - koha-enable error handling
Summary: koha-enable error handling
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Chris Cormack
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 4876
  Show dependency treegraph
 
Reported: 2013-04-23 14:54 UTC by Tomás Cohen Arazi (tcohen)
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:
Circulation function:


Attachments
Bug 10101 - make koha-enable more robust (4.44 KB, patch)
2013-04-23 15:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Signed off patch (4.50 KB, patch)
2013-04-28 19:53 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10101 - Follwup: fix param check (1.02 KB, patch)
2013-04-29 18:19 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Follow up signed off (1.08 KB, patch)
2013-04-29 20:14 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10101 - Follwup: fix param check (1.16 KB, patch)
2013-04-30 07:10 UTC, Mason James
Details | Diff | Splinter Review
Bug 10101 - make koha-enable more robust (4.58 KB, patch)
2013-04-30 07:12 UTC, Mason James
Details | Diff | Splinter Review
Bug 10101 - Follwup: fix param check (1.16 KB, patch)
2013-04-30 07:13 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 (tcohen) 2013-04-23 14:54:55 UTC
Make koha-enable 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 (tcohen) 2013-04-23 15:08:48 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-04-28 19:48:48 UTC
sudo ./debian/scripts/koha-enable sfa
Unknown instance sfa.

sudo ./debian/scripts/koha-enable sfa persona
Unknown instance sfa.
[....] Restarting web server: apache2
ok

sudo ./debian/scripts/koha-enable persona
Instance persona already enabled.
Comment 3 Chris Cormack 2013-04-28 19:53:09 UTC Comment hidden (obsolete)
Comment 4 Mason James 2013-04-29 03:35:57 UTC
(In reply to comment #3)
> Created attachment 17776 [details] [review] [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..."


before...
  # koha-enable 
  Usage: /usr/sbin/koha-enable instancename...


after...
  # koha-enable 
  #
Comment 5 Tomás Cohen Arazi (tcohen) 2013-04-29 18:19:21 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2013-04-29 20:13:28 UTC
sudo ./debian/scripts/koha-enable             
Enables Koha instances.

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

Missing instance name...
Comment 7 Chris Cormack 2013-04-29 20:14:49 UTC Comment hidden (obsolete)
Comment 8 Mason James 2013-04-30 07:10:17 UTC Comment hidden (obsolete)
Comment 9 Mason James 2013-04-30 07:12:08 UTC
Created attachment 17835 [details] [review]
Bug 10101 - make koha-enable more robust

koha-enable now:

- checks for the existence of the instance before any other action on it.
- checks if the instance is already enabled before changing stuff in the config files.
- only reloads apache if it is needed!
- handles more than one instance name as parameter (the code was there, a check for the cardinality of the args prevented it from working).
- documents this behaviour change in the docs
- doesn't break if the provided (invalid) instance name is a prefix/suffix of a real one (added -x to the relevant grep command).

To test:
- Aplpy the patch, build your packages
- Run koha-enable on
  - Non existent instance (try using a prefix or a suffix of an already created one too).
  - Already enabled existent instance name.
  - Disabled instance.

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 10 Mason James 2013-04-30 07:13:35 UTC
Created attachment 17836 [details] [review]
Bug 10101 - Follwup: fix param check

As Mason noted, there was an error on that line.

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 11 Mason James 2013-04-30 07:14:31 UTC
(In reply to comment #10)
> Created attachment 17836 [details] [review] [review]
> Bug 10101 - Follwup: fix param check
> 
> As Mason noted, there was an error on that line.
> 
> 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:53 UTC
This patch has been pushed to master and 3.12.x.
Comment 13 Chris Cormack 2013-05-09 09:21:02 UTC
Pushed to 3.10.x will be in 3.10.6