Bug 11850 - Bulkmarcimport does not append in logfile
Summary: Bulkmarcimport does not append in logfile
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Matthias Meusburger
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-26 15:58 UTC by Matthias Meusburger
Modified: 2015-06-04 23:30 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
proposed patch (900 bytes, patch)
2014-02-26 16:01 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 11850: Fix bulkmarcimport -l option to append to the logfile (958 bytes, patch)
2014-03-07 19:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
proposed patch (2.21 KB, patch)
2014-03-10 15:43 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 11850: Adds a -append option to add data to the logfile (2.36 KB, patch)
2014-03-13 15:54 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 11850: [SIGNED-OFF] Adds a -append option to add data to the logfile (2.38 KB, patch)
2014-03-13 15:55 UTC, Magnus Enger
Details | Diff | Splinter Review
[PASSED QA] Bug 11850: [SIGNED-OFF] Adds a -append option to add data to the logfile (2.57 KB, patch)
2014-04-06 21:07 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Meusburger 2014-02-26 15:58:05 UTC
Bulkmarcimport has the -l option, which allows to write to a logfile.

It does work, however, its behavior is not what we expect from a logfile.

Indeed, bulkmarcimport erases the logfile each time it is executed.

The expected behavior imho is to append the logs to the given file.


Test plan:

1) use bulkmarcimport several times with the same logfile given with -l.
2) the logfile is erased every time. -> KO

Apply the patch, then:

1) use bulkmarcimport several times with the same logfile given with -l. 
2) the logs are correctly appended to the logfile. -> OK
Comment 1 Matthias Meusburger 2014-02-26 16:01:15 UTC Comment hidden (obsolete)
Comment 2 Matthias Meusburger 2014-02-27 13:42:01 UTC
Some concerns have been raised on #koha whether new content for a log file should erase previous content or add to it.

In my opinion, it should add, to prevent data loss.

Managing growing files should be a sysop concern (by using logrotate, for instance).
Comment 3 Kyle M Hall 2014-03-07 19:54:13 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2014-03-07 19:54:47 UTC
From a QA standpoint, if this is controversial, maybe there should be a flag to choose between append vs overwrite for file logging.
Comment 5 Katrin Fischer 2014-03-08 10:53:54 UTC
I think this can't go in as it is. On #koha an optional flag was discussed:
http://irc.koha-community.org/koha/2014-02-26#i_1471750

Setting to "In Discussion". 

I am tending to make this new behaviour depending on a command line option.
Comment 6 Matthias Meusburger 2014-03-10 15:43:04 UTC Comment hidden (obsolete)
Comment 7 Matthias Meusburger 2014-03-10 15:43:57 UTC
The new proposed patch adds a -append option to add to the logfile.
Comment 8 Magnus Enger 2014-03-13 15:54:29 UTC Comment hidden (obsolete)
Comment 9 Magnus Enger 2014-03-13 15:55:21 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2014-04-06 21:07:54 UTC
Created attachment 26846 [details] [review]
[PASSED QA] Bug 11850: [SIGNED-OFF] Adds a -append option to add data to the logfile

Signed-off-by: Magnus Enger <digitalutvikling@gmail.com>
Keeps current behaviour as default.
The -append option is described in the POD and works as expected.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described.
Adding a date/time to the output might
be good, to make it easier to find the entry you were looking for.
Comment 11 Galen Charlton 2014-04-07 15:48:17 UTC
Pushed to master.  Thanks, Matthias!
Comment 12 Fridolin SOMERS 2014-06-19 14:56:25 UTC
Pushed to 3.14.x, will be in 3.14.08