Bug 30350 - Do not hardcode job types
Summary: Do not hardcode job types
Status: RESOLVED DUPLICATE of bug 27783
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 30172
Blocks:
  Show dependency treegraph
 
Reported: 2022-03-24 14:04 UTC by Tomás Cohen Arazi
Modified: 2022-04-29 14:13 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl (1.59 KB, patch)
2022-03-25 13:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl (1.97 KB, patch)
2022-03-28 09:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl (2.02 KB, patch)
2022-04-01 12:35 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-03-24 14:04:39 UTC
Currently background_jobs_worker.pl has a hardcoded list. It should be using a method in Koha::BackgrounJob(s) instead, avoiding the need to maintain this list. Right now it could use Koha::BackgroundJob::type_to_class_mapping.
Comment 1 Tomás Cohen Arazi 2022-03-25 13:06:33 UTC
Created attachment 132246 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Jonathan Druart 2022-03-25 14:10:07 UTC
The point was to have different workers with different job types.
Comment 3 Tomás Cohen Arazi 2022-03-25 15:15:06 UTC
(In reply to Jonathan Druart from comment #2)
> The point was to have different workers with different job types.

Yeah, I get it, but it is not implemented yet, right? Let's talk about it on monday :-D
Comment 4 Jonathan Druart 2022-03-28 09:17:15 UTC
Created attachment 132298 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch
-my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping};
+my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping()};
To remove the following error:
Ambiguous use of %{Koha::BackgroundJob::type_to_class_mapping} resolved to %Koha::BackgroundJob::type_to_class_mapping
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Kyle M Hall 2022-04-01 12:35:39 UTC
Created attachment 132857 [details] [review]
Bug 30350: Avoid hardcoding job types in background_jobs_worker.pl

Currently, background_jobs_worker.pl has a hardcoded list of jobs it can
process. Koha::BackgroundJob already has a method that can be used to
extract such list. Using it would avoid having to maintain two lists,
and it would also make it easier to inject plugin tasks as well.

To test:
1. Apply the patch
2. Restart the koha-worker
3. Run one of the batch operations that make use of the background jobs
   (I used batch item delete)
=> SUCCESS: Things work, no change
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch
-my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping};
+my @job_types = keys %{Koha::BackgroundJob::type_to_class_mapping()};
To remove the following error:
Ambiguous use of %{Koha::BackgroundJob::type_to_class_mapping} resolved to %Koha::BackgroundJob::type_to_class_mapping
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Tomás Cohen Arazi 2022-04-01 14:41:32 UTC
I would like to take this out of the RM queue for now, as work on bug 27783 might make this require some changes.

If someone (including me) finds this is still relevant, let's make it PQA again.

Thanks Kyle!
Comment 7 Martin Renvoize 2022-04-29 14:13:16 UTC
We opted for Queues in bug 27783.. marking as Resolved Fixed

*** This bug has been marked as a duplicate of bug 27783 ***