https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21073
--- Comment #49 from Martin Renvoize <martin.renvo...@ptfs-europe.com> --- Comment on attachment 89801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89801 Bug 21073: Improve plugin performance Review of attachment 89801: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=21073&attachment=89801) ----------------------------------------------------------------- A few, hopefully minor, bits of feedback.. Failing QA whilst I await a reply. ::: Koha/Plugins.pm @@ +73,5 @@ > my $method = $params->{method}; > my $req_metadata = $params->{metadata} // {}; > > + my $dbh = C4::Context->dbh; > + my $plugin_classes = $dbh->selectcol_arrayref('SELECT > DISTINCT(plugin_class) FROM plugin_methods'); Why do we mix old dbh calls with dbic calls in this new code? Also, can we not skip a series of DB calls that are found in the loop by filtering on $method if it exists before running through the loop? @@ +119,4 @@ > > my $plugin = $plugin_class->new({ enable_plugins => > $self->{'enable_plugins'} }); > > + Koha::Plugins::Methods->search({ plugin_class => $plugin_class > })->delete(); Can't make my mind up as to whether we should really rebuilding the whole db table with every/any install/upgrade of any plugin... ::: Koha/Plugins/Handler.pm @@ +63,5 @@ > my $params = $args->{'params'}; > > + my $has_method = Koha::Plugins::Methods->search({ plugin_class => > $plugin_class, plugin_method => $plugin_method })->count(); > + if ( $has_method ) { > + load $plugin_class; Is it ever possible for 'load' to fail here and if so should we catch and warn about it?.. We seem to have effectively removed a warning from the prior code.. I'm wondering if we may ever get a case where the plugin exists in the DB but has been deleted from the filesystem. @@ +68,2 @@ > my $plugin = $plugin_class->new( { cgi => $cgi, enable_plugins => > $args->{'enable_plugins'} } ); > + my @return = $plugin->$plugin_method( $params ); The above line is never referenced right? ::: plugins/plugins-upload.pl @@ +87,4 @@ > $template->param( ERRORS => [ \%errors ] ); > output_html_with_http_headers $input, $cookie, > $template->output; > exit; > + } else { 'unless else' is a weird looking construct.. as we're calling 'exit' in the unless block we can just call the 'Koha::Plugins->new()->InstallPlugins' method below the block and not in it's own else block. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/