[Bug 28606] New: Replace $ENV{DEBUG} and $DEBUG with Koha::Logger->debug
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Bug ID: 28606 Summary: Replace $ENV{DEBUG} and $DEBUG with Koha::Logger->debug Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org Depends on: 28572 Same as bug 28572 for $ENV{DEBUG} and $DEBUG. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28572 [Bug 28572] Replace C4::Debug with Koha::Logger->debug -- 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=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |jonathan.druart+koha@gmail. |ity.org |com -- 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=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au, | |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122281&action=edit Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122281|0 |1 is obsolete| | --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 122321 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122321&action=edit Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Another nice bit of cleanup here. There are a few cases of "$debug and warn..." lines, but they appear to hard code setting `$debug = 0` internally and can be handled in another report I think. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Looking here now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=28606 --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 122327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122327&action=edit Bug 28620: Remove trailing space when logging with log4perl Trivial change. Do sed -i -r -e'/log4perl/ s/\s%n$/%n/' on the log4perl configs. Test plan: Update your own config. Trigger some logging and check that logfile. 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=28606 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Oops -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 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=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122327|0 |1 is obsolete| | --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Comment on attachment 122327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122327 Bug 28620: Remove trailing space when logging with log4perl Not here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28620 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- +#warn "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", join ' ', @mapkeys, "\n"; @mapkeys = grep {defined $mapping{$_}->{is}} @mapkeys; -$debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; +#warn "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; Any plans here ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + warn #"Finished \%memberhash has ", scalar(keys %memberhash), " keys\n", "Referencing \%mapping with ", scalar(keys %mapping), " keys\n"; Typo typo -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just a side note: - if ($pkt =~ /AZ(....)$/) { - $debug and warn "verify_cksum: sum ($1) detected"; - } else { + unless ($pkt =~ /AZ(....)$/) { I have something with unless. When the ifs go down and the unlesses go up, Koha gets more complicated ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- -$ENV{DEBUG} = 1; -warning_like { in_iprange("192.168.1.1/36") } - qr/cidrlookup failed for/, - 'noisy simple invalid ip range/36 with remote ip in it'; Why removed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122321|0 |1 is obsolete| | --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 122330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122330&action=edit Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks good to me, noting a few minor points on BZ. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Martin Renvoize from comment #3)
Another nice bit of cleanup here.
There are a few cases of "$debug and warn..." lines, but they appear to hard code setting `$debug = 0` internally and can be handled in another report I think.
Yes, it is expected. It's from scripts, not module. I thought it would be good to keep them. (In reply to Marcel de Rooy from comment #8)
+#warn "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", join ' ', @mapkeys, "\n"; @mapkeys = grep {defined $mapping{$_}->{is}} @mapkeys; -$debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; +#warn "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n";
Any plans here ?
I don't think they should be logged, but they sounded useful, in case someone need to debug LDAP. (In reply to Marcel de Rooy from comment #9)
+ warn #"Finished \%memberhash has ", scalar(keys %memberhash), " keys\n", "Referencing \%mapping with ", scalar(keys %mapping), " keys\n";
Typo typo
Oops, will fix. (In reply to Marcel de Rooy from comment #10)
Just a side note: - if ($pkt =~ /AZ(....)$/) { - $debug and warn "verify_cksum: sum ($1) detected"; - } else { + unless ($pkt =~ /AZ(....)$/) {
I have something with unless. When the ifs go down and the unlesses go up, Koha gets more complicated ;)
I like unless :) (In reply to Marcel de Rooy from comment #11)
-$ENV{DEBUG} = 1; -warning_like { in_iprange("192.168.1.1/36") } - qr/cidrlookup failed for/, - 'noisy simple invalid ip range/36 with remote ip in it';
Why removed?
Heh, it was not trivial to keep as we are now using Koha::Logger, and didn't think very useful to keep anyway. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #122330|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122331&action=edit Bug 28606: Remove $DEBUG and $ENV{DEBUG} We should remove the debug statements or use Koha::Logger when we want to keep it. Test plan: Confirm that occurrences of remaining occurrences of DEBUG need to be kept (historical scripts for instance) Confirm that the occurrences removed by this patch can be removed Confirm that the occurrences replaced by Koha::Logger are correct Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Looks good to me, noting a few minor points on BZ. JD amended patch: replace "warn #Finished" with "#warn Finished", and put the statement on a single line -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.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=28606 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- t/Auth_with_shibboleth.t is failing 12:06:13 koha_1 | Log::Log4perl configuration looks suspicious: No loggers defined at /usr/share/perl5/Log/Log4perl/Config.pm line 325. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Bug 28606 depends on bug 28572, which changed state. Bug 28572 Summary: Replace C4::Debug with Koha::Logger->debug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28572 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Enhancement not pushed to 21.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 122796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=122796&action=edit Bug 28606: Remove warning from C4::Auth 887 [WARN] Use of uninitialized value in sprintf at /kohadevbox/koha/C4/Auth.pm line 887. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Follow-up pushed to master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- *** Bug 28699 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org