https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22417
--- Comment #65 from Jonathan Druart <[email protected]> --- (In reply to David Cook from comment #63) > (In reply to Jonathan Druart from comment #60) > > Yes sure, I agree. I will implement that as soon as this is PQA. > > The latest work still seems like work-in-progress to me? Yes and no. It's ready for testing. I will address QA comment once this will hit the QA process. We will certainly deal with other additions on separate bug reports. > - Connection details and credentials are still hard-coded into > Koha::BackgroundJob, and there is no way of passing in a virtual host (which > we might not use out of the box, but it would be good to build in the > support) > - For instance, koha_worker.pl should be moved to > "./misc/bin/koha_worker.pl". It's in misc/background_jobs_worker.pl the koha_worker.pl script is part of a "DO NOT PUSH" patch and is there to understand what's going on. Please read the test plan from comment 30. > - I was thinking the "namespace" for the queues should use the database name > instead, since non-Debian installs may or may not have correctly set > memcached_namespace. Yes, it's in the comment: # This namespace is wrong, it must be a vhost instead. # But to do so it needs to be created on the server => much more work when a new Koha instance is created. # Also, here we just want the Koha instance's name, but it's not in the config... # Picking a random id (memcached_namespace) from the config That will be addressed on bug 25674. > - I don't think there's any service to start up koha_worker.pl? There is a patch titled "Add debian script koha-worker". > - I'm curious why koha_worker.pl doesn't use the /queue prefix (as suggested > by https://www.rabbitmq.com/stomp.html). I think it works without it, as I > have tested your patches, but I'm curious. It's also in Net::Stomp's POD, so I guess it's a good pattern to follow. I will submit a follow-up. > - Could we use a "/" instead of a "-" for the queue destination. I think > that would be more conventional. Agreed. > - Koha::BackgroundJob loads C4::MarcModificationTemplates and C4::Biblio but > doesn't use them. Yes, leftover. Thanks! > I'd really like to see this code be plugin friendly from Day 1, which I > think could be done with some updates to koha_worker.pl and > Koha::BackgroundJob. At the moment, koha_worker.pl and Koha::BackgroundJob > are hard-coded with 2 job_types. It wouldn't be difficult to move the > job_types somewhere more configurable. It won't from Day 1. I really need a sign from the community that we agree on a first move. Make it more complex now is not a good idea. Once we agreed on that I promise to move all the different background jobs we have in a short term. That will show us some pattern that is repeated and must be refactored. For instance when I rebased yesterday I had to fix a conflict with bug 18127 (see patch "Restore the 'add to list' feature"). It highlights an interesting feature that must be implemented: a post-process hook to display something specific to the job's report. I hard-coded it for now, but that would be something great to implement in the next steps, especially for the plugins. (In reply to David Cook from comment #64) > (In reply to David Cook from comment #62) > > (In reply to Jonathan Druart from comment #60) > > > Yes sure, I agree. I will implement that as soon as this is PQA. > > > > Could you squash your commits and then post a patch here? > > > > If you squash your commits and post a patch here, I'd be happy to > collaborate on this work. I would prefer to keep the history. Checking out a remote branch should not prevent you to collaborate, it's even more easier than attaching the patches from here :) -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] 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/
