[Bug 38104] New: share_usage_with_koha_community.pl: Check between two runs should be a bit lower than 30d
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Bug ID: 38104 Summary: share_usage_with_koha_community.pl: Check between two runs should be a bit lower than 30d Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz
From production data:
HEA preference: 1725159125 select FROM_UNIXTIME( 1725159125 ); | 2024-09-01 04:52:05 | select * from action_logs where timestamp>'2024-10-01' and module='CRONJOBS' and script like '%usage%'; | 7429456 | 2024-10-01 04:52:04 | 0 | CRONJOBS | Run | 116996 | /usr/share/koha/misc/cronjobs/share_usage_with_koha_community.pl | cron | share_usage_with_koha_community.pl | | 7429457 | 2024-10-01 04:52:04 | 0 | CRONJOBS | End | 116996 | /usr/share/koha/misc/cronjobs/share_usage_with_koha_community.pl COMPLETED | cron | share_usage_with_koha_community.pl | So the HEA update was skipped since the script was 1 second too early :) -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Since we run this script from cron.monthly, it would be enough to check for say four weeks. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 172447 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=172447&action=edit Bug 38104: Adjust number of seconds in UsageStats::NeedUpdate Test plan: Check pref UsageStatsLastUpdateTime Run perl -MC4::UsageStats -e'print C4::UsageStats::NeedUpdate()' Verify that it prints 1 only if time interval exceeds 4 weeks. Temporarily clear this pref. Run again. Should always print 1 now. Restore pref value. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |phil@chetcolibrary.org --- Comment #3 from Phil Ringnalda <phil@chetcolibrary.org> --- Isn't that still risky for March 1st being run 2419199 seconds after February 1st? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- Could you be a bit more specific in the test plan how to test this, please? - There is no UsageStatsLastUpdateTime system preference (in main). - Do I apply the patch first? - Do I need to configure Administration > Additional parameters > Share your usage statistics? Shouldn't the commit message have a description, as per the commit message guidelines? (https://wiki.koha-community.org/wiki/Commit_messages#Bug.2Ffeature_descripti...) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Phil Ringnalda from comment #3)
Isn't that still risky for March 1st being run 2419199 seconds after February 1st?
Yeah good point. Currently an issue too.. -- You are receiving this mail because: You are watching all bug changes.
Could you be a bit more specific in the test plan how to test this, please?
- There is no UsageStatsLastUpdateTime system preference (in main). Local prefs. - Do I apply the patch first? - Do I need to configure Administration > Additional parameters > Share your usage statistics? Yeah. Needs to be enabled. Will add a step to insert some last update time in
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to David Nind from comment #4) the test plan.
Shouldn't the commit message have a description, as per the commit message guidelines? (https://wiki.koha-community.org/wiki/Commit_messages#Bug. 2Ffeature_description)
This is about 2 lines of code. So imo covered by the title.. Not uncommon to see. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |BLOCKED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #172447|0 |1 is obsolete| | --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 176674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176674&action=edit Bug 38104: Adjust number of seconds in UsageStats::NeedUpdate The current interval between two HEA updates is hardcoded to 2592000 seconds, i.e. 30 days. This should be a bit lower to compensate for shorter months (Feb) or a small difference in execution time under cron. This patch adjusts the 30 days to 27 days. This allows us to not miss February too. Note that if you set it to 28 days, a few minutes difference could already cause a HEA skip too. Test plan: Enable pref UsageStats. Check pref UsageStatsLastUpdateTime (under Local use). Keep the value X somewhere. (Don't worry if it was empty.) You can convert this value X with: perl -MPOSIX -e 'print POSIX::strftime("%Y-%m-%d %H:%M:%S\n", localtime(X) )' The current time is shown in seconds with: perl -e'print time()' Enter a value like 1234 in UsageStatsLastUpdateTime. This should trigger an update normally. Run perl -MC4::UsageStats -e'print C4::UsageStats::NeedUpdate()' This should print 1. Now put the current time in seconds in the pref (see above). Run perl -MC4::UsageStats -e'print C4::UsageStats::NeedUpdate()' This should print 0. [BONUS] Test with a value now - 26 days, now - 28 days. Temporarily clear the pref. Run again. Should print 1 again. Restore pref value. Disable UsageStats if it was not enabled. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Well, ended up with a 'long' commit message/test plan for this tiny change :) Please test ! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176674|0 |1 is obsolete| | --- Comment #9 from Phil Ringnalda <phil@chetcolibrary.org> --- Created attachment 176702 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176702&action=edit Bug 38104: Adjust number of seconds in UsageStats::NeedUpdate The current interval between two HEA updates is hardcoded to 2592000 seconds, i.e. 30 days. This should be a bit lower to compensate for shorter months (Feb) or a small difference in execution time under cron. This patch adjusts the 30 days to 27 days. This allows us to not miss February too. Note that if you set it to 28 days, a few minutes difference could already cause a HEA skip too. Test plan: Enable pref UsageStats. Check pref UsageStatsLastUpdateTime (under Local use). Keep the value X somewhere. (Don't worry if it was empty.) You can convert this value X with: perl -MPOSIX -e 'print POSIX::strftime("%Y-%m-%d %H:%M:%S\n", localtime(X) )' The current time is shown in seconds with: perl -e'print time()' Enter a value like 1234 in UsageStatsLastUpdateTime. This should trigger an update normally. Run perl -MC4::UsageStats -e'print C4::UsageStats::NeedUpdate()' This should print 1. Now put the current time in seconds in the pref (see above). Run perl -MC4::UsageStats -e'print C4::UsageStats::NeedUpdate()' This should print 0. [BONUS] Test with a value now - 26 days, now - 28 days. Temporarily clear the pref. Run again. Should print 1 again. Restore pref value. Disable UsageStats if it was not enabled. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Phil Ringnalda from comment #9)
Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@gmail.com --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- I don't understand why we have both actually. Either we run the cronjob every day and the script check if it needs to update, or we run the cronjob every month and the script just updates. Should not we simply remove from this NeedUpdate check? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #11)
I don't understand why we have both actually.
Either we run the cronjob every day and the script check if it needs to update, or we run the cronjob every month and the script just updates.
Should not we simply remove from this NeedUpdate check?
Yeah fine with me. I am dropping this report for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|m.de.rooy@rijksmuseum.nl |koha-bugs@lists.koha-commun | |ity.org -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #13 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176957&action=edit [ALTERNATIVE-PATCH] Bug 38104: Always update when hea cronjob is run If the cronjob is configured to run every month we don't need to add another test on the delay in the script itself. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #14 from David Nind <david@davidnind.com> --- The alternative patch makes sense to me (given my limited understanding of any technical issues) - update when the cronjob was last run. Happy to test and sign off (the t/db_dependent/UsageStats.t t/db_dependent/UsageStats.t tests pass before and after). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 177096 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=177096&action=edit Bug 38104: Always update when hea cronjob is run If the cronjob is configured to run every month we don't need to add another test on the delay in the script itself. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Assignee|koha-bugs@lists.koha-commun |jonathan.druart@gmail.com |ity.org | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- SO and QA in one go. Trivial change. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|share_usage_with_koha_commu |share_usage_with_koha_commu |nity.pl: Check between two |nity.pl: Check between two |runs should be a bit lower |runs is not needed |than 30d | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176702|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176957|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.05.00 |25.05.00,24.11.04 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 --- Comment #18 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #19 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38104 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #20 from David Nind <david@davidnind.com> --- Bug fix, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org