Bug 10735 - koha-dump not working
Summary: koha-dump not working
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: Main
Hardware: All Linux
: P5 - low normal (vote)
Assignee: Robin Sheat
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-15 07:22 UTC by Bert Burgemeister
Modified: 2015-06-04 23:23 UTC (History)
8 users (show)

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


Attachments
Bug 10735 - prevent script aborting if backupdir undefined (1.50 KB, patch)
2013-11-19 02:29 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 10735 - prevent script aborting if backupdir undefined (1.74 KB, patch)
2013-12-19 18:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10735 - prevent script aborting if backupdir undefined (1.94 KB, patch)
2013-12-23 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Bert Burgemeister 2013-08-15 07:22:05 UTC
Steps to Reproduce (on Debian, installing via apt-get koha-common):

(1) Use koha-run-backups on an older Koha version (I used 3.10 which has been updated from an even older installation).

(2) koha-restore data and configuration from step (1) into a Koha 3.12 installation.

(3) Run /usr/sbin/koha-dump.


Actual Result: It says "Dumping Koha site [...]:".  Nothing else happens.


Expected Result: Two dump files.


Workaround: Remove the "set -e" line in /usr/sbin/koha-dump


Additional Information:

The older configuration doesn't have a config/backupdir entry.

backupdir="$( xmlstarlet sel -t -v 'yazgfs/config/backupdir' $kohaconfig )"

therefore returns 1 and exits koha-dump.
Comment 1 Liz Rea 2013-08-16 00:37:48 UTC
Did you specify an instance to dump when you ran koha-dump? 

The correct syntax is: koha-dump instancename

Just making sure. :)
Comment 2 Robin Sheat 2013-08-16 08:45:31 UTC
The best solution would be a fallback for the backup path.
Comment 3 Bert Burgemeister 2013-08-18 11:02:20 UTC
(In reply to Robin Sheat from comment #2)
> The best solution would be a fallback for the backup path.

The fallback is already sort of implemented.  It just doesn't work
because xmlstarlet returns non-zero which kills koha-dump immediately
due to its "set -e" setting.
Comment 4 Robin Sheat 2013-09-01 21:53:14 UTC
(In reply to Bert Burgemeister from comment #3)
> The fallback is already sort of implemented.  It just doesn't work
> because xmlstarlet returns non-zero which kills koha-dump immediately
> due to its "set -e" setting.

It'd be better to fix that case with some error handling (or even just '|| true' to hide it if appropriate) than turn off the error aborting script-wide, as that may cause it to try to plow on rather than die if something else goes wrong.
Comment 5 Robin Sheat 2013-11-19 02:29:22 UTC Comment hidden (obsolete)
Comment 6 Robin Sheat 2013-11-19 02:30:50 UTC
Patch applied against master, but I'd expect it to backport cleanly.
Comment 7 Tomás Cohen Arazi 2013-12-19 18:03:54 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2013-12-23 13:57:16 UTC
Created attachment 23784 [details] [review]
Bug 10735 - prevent script aborting if backupdir undefined

Older versions of Koha didn't have <backupdir>...</backupdir> defined in
koha-conf.xml. The koha-dump script is aware of this, and checks to see
if it's there. However, if it's not, xmlstarlet returns a non-0 error
code which causes the script to abort due to running under set -e.

Test plan:
 * Remove backupdir from koha-conf.xml
 * Run koha-dump, notice that it doesn't do backups
 * Apply patch
 * Run koha-dump again, notice that it does do backups

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
The fallback method works as described. It might be made more robust
like checking for dir existence (thinking of older instances upgrades).
But it certaintly belongs to another bug report.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
As Tomas said, it would be better to alert the user if the backupdir tag
does not exist (or contains a nonexistent directory) in the config file.
Comment 9 Jonathan Druart 2013-12-23 13:59:33 UTC
The patch is for master and can be backported to stable branches.
Comment 10 Galen Charlton 2013-12-23 15:57:41 UTC
Pushed to master.  Thanks, Robin!
Comment 11 Fridolin Somers 2013-12-30 10:14:07 UTC
Patch pushed to 3.14.x, will be in 3.14.2.
Comment 12 Tomás Cohen Arazi 2014-01-15 17:49:24 UTC
This patch has been pushed to 3.12.x, will be in 3.12.9.

Thanks Robin!