[Bug 27434] New: Task Scheduler should use classes as job types
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27434 Bug ID: 27434 Summary: Task Scheduler should use classes as job types Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Task Scheduler Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Rather than maintaining a list of types and going through each case we should use the class as the job type so we can call it directly -- 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=27434 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 115164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115164&action=edit Bug 27434: POC -- 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=27434 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Depends on| |22417 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22417 [Bug 22417] Add a task queue -- 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=27434 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 115164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115164 Bug 27434: POC Review of attachment 115164: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=27434&attachment=115164) ----------------------------------------------------------------- ::: Koha/BackgroundJob.pm @@ +137,4 @@
my ( $self, $args ) = @_;
my $job_type = $self->type; + my $class = "Koha::BackgroundJob::${job_type}";
I'd suggest $job_type actually contain the whole "Koha::BackgroundJob::BatchUpdateBiblio". I think that would be more powerful/flexible. It could also make for fewer typos. ::: Koha/BackgroundJob/BatchUpdateAuthority.pm @@ +43,4 @@
=cut
sub job_type { + return 'BatchUpdateAuthority';
If we actually use the entire Koha::BackgroundJob::BatchUpdateAuthority string for the job_type, then we could replace this with 'return __PACKAGE__;' ::: misc/background_jobs_worker.pl @@ +28,5 @@
warn sprintf "Cannot connect to the message broker, the jobs will be processed anyway (%s)", $_; };
+# FIXME Job types should be stored in a table so that plugins can register new job types as well +my @job_types = qw( BatchUpdateBiblio BatchUpdateAuthority );
Honestly, I don't think that we actually even need to store job_types. Something that could be interesting though is specifying what queues the background_jobs_worker.pl will subscribe to. This could be useful in terms of future scaling of workers. For instance, you might want only 1 worker for a lot of background tasks, but you might want a few for particularly busy queues. -- 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=27434 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Overall, I'm very pro this idea. In fact, class names are what I have already been using for my local Koha::MQ stuff prior to to pushing of Bug 22417. -- 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=27434 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- To be clear, I implemented it like that to provide an "allow list" of tasks we can executed. Otherwise we will potentially execute anything we don't have the hand on it. However we may want to assume that if the DB is corrupted, it's too late already... -- 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=27434 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #4)
To be clear, I implemented it like that to provide an "allow list" of tasks we can executed. Otherwise we will potentially execute anything we don't have the hand on it.
However we may want to assume that if the DB is corrupted, it's too late already...
I suppose that's a case of security vs convenience. Having an allow list would be more secure, but less convenient - especially for plugins. What are the potential threats? Malicious Perl plugins? Other parts of the system sending malicious messages to RabbitMQ? Malicious Perl plugins are already a problem. Malicious messages would have to be extremely well crafted and even then... unlikely to do any harm especially if we used a hook like "run_koha_background_task". -- 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=27434 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=27434 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Totally onboard with this.. thanks for opening the bug -- 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=27434 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28413 -- 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=27434 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30350 Status|NEW |In Discussion --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I filed bug 30350, which implements one line that was present on this bug (I didn't know about). The one removing the codes allow-list from background_job_worker.pl and reusing Koha::BackgroundJob::type_to_class_mapping instead. I think, overall, having an intermediate method that provides an allow-list is good. Inside that method I expect to query for plugin-implemented tasks to be added on this 'core' mapping we are building for core-defined tasks. The last few days I've filed several bugs for polishing the area. Maybe we can delay this enhancement until it is clear how much we gain/loose. -- 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=27434 Bug 27434 depends on bug 22417, which changed state. Bug 22417 Summary: Add a task queue https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22417 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- 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=27434 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Task Scheduler should use |Background Jobs should use |classes as job types |classes as job types -- 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=27434 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=1993 -- 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=27434 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |35092 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35092 [Bug 35092] [OMNIBUS] Remaining background job/worker issues -- 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=27434 Michaela Sieber <michaela.sieber@kit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |clemens.tubach@kit.edu, | |michaela.sieber@kit.edu, | |raphael.straub@kit.edu -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org