http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |6911 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- I wasn't able to apply this patch on master: fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt). -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #5 from Marc Véron <veron@veron.ch> --- (In reply to Frédéric Demians from comment #3)

I wasn't able to apply this patch on master:

fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt).

Hi Frédéric, thanks for testing. The patch depends on Bug 6911 (has changes to viewlog.tt), my patch should apply on top of this bug. I rebased 2 more conflicts after applying 6911. Re-tested applying, works for me on current master. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #7 from Marc Véron <veron@veron.ch> --- Created attachment 37707 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37707&action=edit [Follow-up]Bug 13889 Add syspref to sysprefs.sql and updatedatabase.pl This patch adds the syspref CronjobLog to sysprefs.sql and updatedatabase.pl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37679|0 |1 is obsolete| | Attachment #37707|0 |1 is obsolete| | --- Comment #8 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 37711 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37711&action=edit Bug 13889 - Add information about cron jobs to system log This patch adds information about cron jobs performed and make it viewable under Home > Tools > Logs ("Browse system logs") To test: Apply patch - Got to system preferences and set 'CronjobLog' to: [Log] information from cron jobs. - Run some cron jobs - Go to Home > Tools > Logs - Verify that you have a selection 'Cron jobs' in drop-down 'Module'. Select it with Action "All" and Submit. - Output should show Date/time and info about Cron jobs Rebased to work on top of Bug 6911 (conflict in viewlog.tt) /MV Rebased after applying patch for Bug 6911 /MV Conflicts resolved: misc/cronjobs/overdue_notices.pl misc/cronjobs/cleanup_database.pl Signed-off-by: Frederic Demians <f.demians@tamil.fr> - Merge both patches, and fix updatedatabase.pl - Works as described. Provide intersting feedback from cronjob scripts. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37711|0 |1 is obsolete| | --- Comment #10 from Marc Véron <veron@veron.ch> --- Created attachment 38116 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38116&action=edit Bug 13889 - Add information about cron jobs to system log This patch adds information about cron jobs performed and make it viewable under Home > Tools > Logs ("Browse system logs") To test: Apply patch - Got to system preferences and set 'CronjobLog' to: [Log] information from cron jobs. - Run some cron jobs - Go to Home > Tools > Logs - Verify that you have a selection 'Cron jobs' in drop-down 'Module'. Select it with Action "All" and Submit. - Output should show Date/time and info about Cron jobs Rebased to work on top of Bug 6911 (conflict in viewlog.tt) /MV Rebased after applying patch for Bug 6911 /MV Conflicts resolved: misc/cronjobs/overdue_notices.pl misc/cronjobs/cleanup_database.pl Signed-off-by: Frederic Demians <f.demians@tamil.fr> - Merge both patches, and fix updatedatabase.pl - Works as described. Provide intersting feedback from cronjob scripts. -- Modified version taking in account syspref CronJobLog. Handling simplified by introducing a convenience sub cronlogaction in C4/Log.pm /MV -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |mtompset@hotmail.com --- Comment #13 from M. Tompsett <mtompset@hotmail.com> --- Koha QA tools fails related to my comment #11. And a tab character on line 37 of C4/Log.pm -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #15 from Marc Véron <veron@veron.ch> --- Reset to 'Needs Signoff'. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #17 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38228 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38228&action=edit Bug 13889: Added Test Coverage Previous commit was lacking test coverage of new function. TEST PLAN --------- 1) Apply all the patches 2) prove -v t/db_dependent/Log.t -- should be success. 3) koha qa test tools -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38228|0 |1 is obsolete| | --- Comment #19 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 38230 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38230&action=edit Bug 13889: Added Test Coverage Previous commit was lacking test coverage of new function. TEST PLAN --------- 1) Apply all the patches 2) prove -v t/db_dependent/Log.t -- should be success. 3) koha qa test tools -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #21 from Marc Véron <veron@veron.ch> --- Created attachment 38236 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38236&action=edit [Signed-off] Bug 13889: Added Test Coverage Previous commit was lacking test coverage of new function. TEST PLAN --------- 1) Apply all the patches 2) prove -v t/db_dependent/Log.t -- should be success. 3) koha qa test tools Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #23 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Jonathan Druart from comment #22)

Created attachment 38239 [details] [review] Bug 13889: Log with parameters

Marc, What do you think of this patch? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #25 from Marc Véron <veron@veron.ch> --- (In reply to Jonathan Druart from comment #23)

(In reply to Jonathan Druart from comment #22)

...

Created attachment 38239 [details] [review] [review] Bug 13889: Log with parameters

Marc, What do you think of this patch?

Jonatan, I like the idea. I was thinking about adding some more detailed information to the action log before and I planned to expand it after this patch set is pushed. First consideration: With the patch, I have the following problem: Call of cronlogaction() without param does not display the path: 2015-04-21 15:24:09 0 Cron jobs Run 0 Call of cronlogaction(join ' ', $0, @ARGV) displays the path and the params: 2015-04-21 15:24:09 0 Cron jobs Run 0 misc/cronjobs/overdue_notices.pl -n -csv IMO cronlogaction() (without param) should behave as before. Second consideration: I think it would be better tho have a string as param and to prepend it with caller(0))[1] or whatever it needs to display the script's name. (That is what I was planning as a next step.) With a string param it is up to the logscript's author to decide what to communicate in the logs (in a human readable form), e.g. not only the params but their defaults as well if no params are passed. That brings me to the third consideration: The original idea is that the log is written after all of the preconditions. If the script dies before doing anything "real", no log is written. Otherwise we have an entry in the logs for a cronjob that was called but did not do anything, and users think that everything is OK. With the string param mentioned above the author of the log file then can use logging for more sophisticated things (if necessary), e.g. - cronlogaction("start") - do the preconditions and log some information if it dies: cronlogaction("fail: ..reason...)" - cronlogaction("execute with: ...intersting params / defaults...") - cronlogaction("success") or cronlogaction("success: ...interesting results...") As a minimum, cronlogaction should be called as soon the script does the real work (that's what my patches to the cronjobs do at the moment), in the sense of "Hi, I was here and did something" So I propose to add the string parameter and, for this first step, leave the calls in the cron scripts as is. What do you think? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #26 from Marc Véron <veron@veron.ch> --- Created attachment 38283 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38283&action=edit [Follow-up] Provide parameter $infos in cronlogaction() (Alternative for patch Bug 13889: Log with parameters) To test: - Apply patches interactive and skip following patch: Bug 13889: Log with parameters - Tweak a cron job in order to call cronlogaction twice, one with a param at the beginning of the cron ob, e.g. cronlogaction("This is my message") and one without param. - Run this cron job. Result: You should have two entries in the log, one with and one without additional information after the path and name of the calling script. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #38229|0 |1 is obsolete| | Attachment #38236|0 |1 is obsolete| | Attachment #38239|0 |1 is obsolete| | Attachment #38283|0 |1 is obsolete| | --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 38406 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38406&action=edit [PASSED QA] Bug 13889 - Add information about cron jobs to system log This patch adds information about cron jobs performed and make it viewable under Home > Tools > Logs ("Browse system logs") To test: Apply patch - Got to system preferences and set 'CronjobLog' to: [Log] information from cron jobs. - Run some cron jobs - Go to Home > Tools > Logs - Verify that you have a selection 'Cron jobs' in drop-down 'Module'. Select it with Action "All" and Submit. - Output should show Date/time and info about Cron jobs Rebased to work on top of Bug 6911 (conflict in viewlog.tt) /MV Rebased after applying patch for Bug 6911 /MV Conflicts resolved: misc/cronjobs/overdue_notices.pl misc/cronjobs/cleanup_database.pl Signed-off-by: Frederic Demians <f.demians@tamil.fr> - Merge both patches, and fix updatedatabase.pl - Works as described. Provide intersting feedback from cronjob scripts. -- Modified version taking in account syspref CronJobLog. Handling simplified by introducing a convenience sub cronlogaction in C4/Log.pm /MV Amended to take in account comments #11, #12, #13 /MV http://bugs.koha-community.org/show_bug.cgi?id=13899 Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #29 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 38408 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=38408&action=edit [PASSED QA] Provide parameter $infos in cronlogaction() (Alternative for patch Bug 13889: Log with parameters) To test: - Apply patches interactive and skip following patch: Bug 13889: Log with parameters - Tweak a cron job in order to call cronlogaction twice, one with a param at the beginning of the cron ob, e.g. cronlogaction("This is my message") and one without param. - Run this cron job. Result: You should have two entries in the log, one with and one without additional information after the path and name of the calling script. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 --- Comment #31 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, Have you seen the discussion before? What's your opinion on it? See from comment 22 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13889 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=6810 -- You are receiving this mail because: You are watching all bug changes.