A warn without a prepended $debug is causing floodiness when NorwegianPatronDB is not set.
Though I can't see how to trigger it, except by deleting ~/kohaclone/Koha/NorwegianPatronDB.pm, there probably should be a $debug prepended on the warn.
Created attachment 47881 [details] [review] Bug 15795: C4/Members.pm is floody (Norwegian Patron DB) From the mailing list: "I'm working on 3.22.02 and my logs are FILLED with the following warnings: Members.pm: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/acqui/booksellers.pl admin-home.pl: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/admin/preferences.pl" I agree. Looking at C4/Members.pm other warns are all prepended with $debug. TEST PLAN --------- 1) rm Koha/NorwegianPatronDB.pm -- quickest and dirtiest way to get the load to fail. 2) echo > ~/koha-dev/var/log/koha-error_log -- so it will be easy to spot the log changes. -- back up the log file if you really want it. 3) log into the staff client and go to patrons. -- Check the log file. "Unable to load ..." will be there. 4) echo > ~/koha-dev/var/log/koha-error_log 5) sudo vi /etc/apache2/sites-enabled/{your koha site file} -- Find your staff client entry in your apache configuration file and add: SetEnv DEBUG 1 6) sudo service apache2 restart 7) apply the patch 8) refresh the patron page -- "Unable to load..." will still be there. 9) change the DEBUG value in your apache config to 0 10) sudo sevice apache2 restart 11) echo > ~/koha-dev/var/log/koha-error_log 12) refresh the patron page -- "Unable to load..." will NOT be there. 13) git checkout origin/master -- Koha/NorwegianPatronDB.pm -- to bring it back. 14) refresh the patron page -- "Unable to load..." will still not be there. 15) run koha qa test tools
See the conversation on Nabble: http://koha.1045719.n5.nabble.com/Introduce-the-use-of-Grunt-or-Gulp-tp5873339p5873842.html Sadly, it got inter-threaded. The dangers of just replying in order to create a new thread. :(
How about getting rid of both the if and the warn? So from this: use Module::Load::Conditional qw( can_load ); if ( ! can_load( modules => { 'Koha::NorwegianPatronDB' => undef } ) ) { warn "Unable to load Koha::NorwegianPatronDB"; } to this: use Module::Load::Conditional qw( can_load ); can_load( modules => { 'Koha::NorwegianPatronDB' => undef } );
(In reply to Magnus Enger from comment #4) > How about getting rid of both the if and the warn? So ... this: > > use Module::Load::Conditional qw( can_load ); > can_load( modules => { 'Koha::NorwegianPatronDB' => undef } ); While functional, I think warns are a good thing. I would rather keep the potential to warn, hence my patch.
Created attachment 47902 [details] [review] [SIGNED-OFF] Bug 15795: C4/Members.pm is floody (Norwegian Patron DB) From the mailing list: "I'm working on 3.22.02 and my logs are FILLED with the following warnings: Members.pm: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/acqui/booksellers.pl admin-home.pl: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/admin/preferences.pl" I agree. Looking at C4/Members.pm other warns are all prepended with $debug. TEST PLAN --------- 1) rm Koha/NorwegianPatronDB.pm -- quickest and dirtiest way to get the load to fail. 2) echo > ~/koha-dev/var/log/koha-error_log -- so it will be easy to spot the log changes. -- back up the log file if you really want it. 3) log into the staff client and go to patrons. -- Check the log file. "Unable to load ..." will be there. 4) echo > ~/koha-dev/var/log/koha-error_log 5) sudo vi /etc/apache2/sites-enabled/{your koha site file} -- Find your staff client entry in your apache configuration file and add: SetEnv DEBUG 1 6) sudo service apache2 restart 7) apply the patch 8) refresh the patron page -- "Unable to load..." will still be there. 9) change the DEBUG value in your apache config to 0 10) sudo sevice apache2 restart 11) echo > ~/koha-dev/var/log/koha-error_log 12) refresh the patron page -- "Unable to load..." will NOT be there. 13) git checkout origin/master -- Koha/NorwegianPatronDB.pm -- to bring it back. 14) refresh the patron page -- "Unable to load..." will still not be there. 15) run koha qa test tools Signed-off-by: Philippe Blouin <philippe.blouin@inlibro.com>
Although I would prefer Magnus' code snippet overall, because it's less ugly than the big "if" that gives too much importance to that feature IMO, adding $debug is following the standard and expectations. I like standards...
(In reply to Blou from comment #7) > Although I would prefer Magnus' code snippet overall, > because it's less ugly -- totally agree. > than the big "if" that gives too much importance to that feature IMO, adding > $debug is following the standard and expectations. > > I like standards... And the discussion on development mailing list may end up turning this functionality into a plug-in in the future, but until then, this suffices. +1 to future plugin. :)
Created attachment 47948 [details] [review] Bug 15795: C4/Members.pm is floody (Norwegian Patron DB) From the mailing list: "I'm working on 3.22.02 and my logs are FILLED with the following warnings: Members.pm: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/acqui/booksellers.pl admin-home.pl: Unable to load Koha::NorwegianPatronDB at /blurred/C4/Members.pm line 46., referer: http://blurred/cgi-bin/koha/admin/preferences.pl" I agree. Looking at C4/Members.pm other warns are all prepended with $debug. TEST PLAN --------- 1) rm Koha/NorwegianPatronDB.pm -- quickest and dirtiest way to get the load to fail. 2) echo > ~/koha-dev/var/log/koha-error_log -- so it will be easy to spot the log changes. -- back up the log file if you really want it. 3) log into the staff client and go to patrons. -- Check the log file. "Unable to load ..." will be there. 4) echo > ~/koha-dev/var/log/koha-error_log 5) sudo vi /etc/apache2/sites-enabled/{your koha site file} -- Find your staff client entry in your apache configuration file and add: SetEnv DEBUG 1 6) sudo service apache2 restart 7) apply the patch 8) refresh the patron page -- "Unable to load..." will still be there. 9) change the DEBUG value in your apache config to 0 10) sudo sevice apache2 restart 11) echo > ~/koha-dev/var/log/koha-error_log 12) refresh the patron page -- "Unable to load..." will NOT be there. 13) git checkout origin/master -- Koha/NorwegianPatronDB.pm -- to bring it back. 14) refresh the patron page -- "Unable to load..." will still not be there. 15) run koha qa test tools Signed-off-by: Philippe Blouin <philippe.blouin@inlibro.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to Master - Should be in the May 2016 release. (Just a note one of the previous patches wasn't obsolete - I followed up on that). Thanks!
Patch pushed to 3.22.x, will be in 3.22.4
This patch has been pushed to 3.20.x, will be in 3.20.9.