https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22417
--- Comment #156 from Marcel de Rooy <m.de.r...@rijksmuseum.nl> --- QA Comment: Great to see us moving to a message queue in Koha ! Nice start. And surely things to discuss ;) The UI changes are not really the most relevant imo. We could start as an experimental feature too if needed (in that case we should not touch batch mod). But we should add imo some configuration options (see below, setting up vhost, user, etc.) And perhaps even more important and already commented on a bit, we are adding a task queue here with a message queue. Obviously, we could send other stuff into the MQ; so this might become a problem later on. And sounding a bit nasty, but not meant to be: We are saving the data into a background job table as well as sending it to MQ. Aren't we being redundant here? Actually, it looks like our background job could just hook to the background jobs table and process what is left there. So what is the real benefit here in this implementation of the MQ exactly? Detailed comments: my $stomp = Net::Stomp->new( { hostname => 'localhost', port => '61613' } ); hardcoded port number / we should probably also let rabbitmq run in its own container, so replace localhost too: move to koha-conf No disconnect ? Memory issues when not doing so? $stomp->connect( { login => 'guest', passcode => 'guest' } ); hmm; isnt this the place for vhosts to come in ? just as vhost, a user should have been created? / hardcoded: move to koha-conf or separate conf? # Picking a random id (memcached_namespace) from the config my $namespace = C4::Context->config('memcached_namespace'); Shouldnt we configure a koha vhost during install (koha-create) and indeed pick that from koha-conf or so ? Why not use config/database instead? Normally it looks like koha_instance Not sure btw if using the term namespace is confusing here; or just a queue prefix ? Add Koha::BackgroundJob::BatchUpdateBiblio Not sure if we need that. The crux is here only four lines: my $record = C4::Biblio::GetMarcBiblio({ biblionumber => $biblionumber }); C4::MarcModificationTemplates::ModifyRecordWithTemplate( $mmtid, $record ); my $frameworkcode = C4::Biblio::GetFrameworkCode( $biblionumber ); C4::Biblio::ModBiblio( $record, $biblionumber, $frameworkcode ); The rest of the code is more or less overhead which perhaps better could be moved to the general module. The module should care for iterating too. So if we could pass a sub or method to a general process method along with a params hash here including biblionumber and marc_template_id, it might be enough? Similar remarks for enqueue. Would reduce a number of submodules. So we should enqueue: Koha::Backgroundjob->new({ type => 'batch_record_mod', ... })->enqueue({ mmtid => $mmtid, records => $records }) And process with Koha::Backgroundjobs->process({ type => ... }) where the process method iterates over the records and associates type with e.g. Koha::Biblios::batch_update [four lines above] etc. installer/data/mysql/atomicupdate/bug_15032.perl Wrong bug number diff koha_worker.pl misc/background_jobs_worker.pl Why did you add koha_worker.pl? We already run the misc script ? No tests anywhere [yet]? Ok :) -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://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/