Summary: | Stop warns from advance_notices.pl if not running in verbose mode | ||
---|---|---|---|
Product: | Koha | Reporter: | Hayley Pelham <hayleypelham> |
Component: | Command-line Utilities | Assignee: | Hayley Pelham <hayleypelham> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | minor | ||
Priority: | P5 - low | CC: | andrew, fridolin.somers, kyle, m.de.rooy, robin, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29928 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
22.05.00,21.11.03,21.05.11
|
Circulation function: | |
Attachments: |
Bug 29054: Add verbose check for warn in advance_notices.pl
Bug 29054: Add verbose check for warn in advance_notices.pl Bug 29054: Add verbose check for warn in advance_notices.pl |
Description
Hayley Pelham
2021-09-20 04:18:14 UTC
Created attachment 125086 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT I agree with that Created attachment 127680 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> (In reply to Hayley Pelham from comment #0) > When the advance_notices.pl cronjob is run and the > EnhancedMessagingPreferences syspref is set to don't allow, we get the > following warning: > > The "EnhancedMessagingPreferences" syspref is off. > Therefore, it is unlikely that this script will actually produce any > messages to be sent. > To change this, edit the "EnhancedMessagingPreferences" syspref. > > The advance_notices.pl script help says: > > -v Verbose. Without this flag set, only fatal errors are reported. Not sure if we should remove a warn here or edit the above description? Removing fatal or replacing it by a less severe term? Needs feedback. I agree with only giving this note with -v as at the moment if you have scheduled this on a server with multiple instances where the pref is turned off intentionally, it creates quite a bit of noise. It's not a fatal one I think, so would fit the description of -v, just referring to a configuration issue. Setting back to Signed off after feedback from Katrin - thanks Katrin! (In reply to Katrin Fischer from comment #5) > I agree with only giving this note with -v as at the moment if you have > scheduled this on a server with multiple instances where the pref is turned > off intentionally, it creates quite a bit of noise. > > It's not a fatal one I think, so would fit the description of -v, just > referring to a configuration issue. I dont oppose pushing it for the sake of multiple instances (where some instances would have enhanced messaging and others might not). But suppressing a warn that tells you that you are running a script that wouldnt do anything, is imo generally not what we should do. Created attachment 129242 [details] [review] Bug 29054: Add verbose check for warn in advance_notices.pl To test: 1) Set EnhancedMessagingPreferences to Don't allow 2) In the koha-shell, run misc/cronjobs/advance_notices.pl -c 3) Note that you see the warning "The "EnhancedMessagingPreferences" syspref is off... etc." 4) Apply the patch and restart services 5) In the koha-shell, run misc/cronjobs/advance_notices.pl -c and note the warning no longer shows 6) Still in the shell, run misc/cronjobs/advance_notices.pl -c -v and note the warning does show Sponsored-by: Catalyst IT Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Marcel de Rooy from comment #7) > (In reply to Katrin Fischer from comment #5) > > I agree with only giving this note with -v as at the moment if you have > > scheduled this on a server with multiple instances where the pref is turned > > off intentionally, it creates quite a bit of noise. > > > > It's not a fatal one I think, so would fit the description of -v, just > > referring to a configuration issue. > > I dont oppose pushing it for the sake of multiple instances (where some > instances would have enhanced messaging and others might not). But > suppressing a warn that tells you that you are running a script that wouldnt > do anything, is imo generally not what we should do. We have other examples of script. membership_expiry.pl and preference MembershipExpiryDaysNotice: https://git.koha-community.org/Koha-community/Koha/src/commit/e653effe5783e149f5ab69f388aa5509db454ef4/misc/cronjobs/membership_expiry.pl#L182 Using -v is a very common way to know what a script does ;) Created Bug 29928 This isnt enhancement, I change to minor to encourage backport Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.11 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |