Bug 32394 - Long tasks queue is never used
Summary: Long tasks queue is never used
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords: rel_22_11_candidate
Depends on: 27783
Blocks: 32393
  Show dependency treegraph
 
Reported: 2022-12-02 13:39 UTC by Jonathan Druart
Modified: 2023-12-28 20:43 UTC (History)
6 users (show)

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


Attachments
Bug 32394: Restore long tasks queue (7.69 KB, patch)
2022-12-02 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32394: Restore long tasks queue (7.74 KB, patch)
2022-12-06 12:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32394: Restore long tasks queue (7.81 KB, patch)
2022-12-09 10:43 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32394: Add tests (1.15 KB, patch)
2022-12-14 08:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32394: Add tests (1.20 KB, patch)
2022-12-14 08:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32394: Regression tests (27.79 KB, patch)
2022-12-14 17:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32394: Restore long tasks queue (7.85 KB, patch)
2022-12-14 17:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32394: Add tests (1.26 KB, patch)
2022-12-14 17:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32394: (follow-up) Add param for Koha::BackgroundJob::StageMARCForImport->enqueue (1.58 KB, patch)
2022-12-16 12:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32394: [22.05.x] Regression tests (19.12 KB, patch)
2023-02-01 16:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32394: [22.05.x] Restore long tasks queue (5.07 KB, patch)
2023-02-01 16:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32394: [22.05.x] Add tests (1.27 KB, patch)
2023-02-01 16:13 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2022-12-02 13:39:38 UTC
We are sending "queue" as parameter, but "job_queue" is retrieved in Koha::BackgroundJob->enqueue...
Comment 1 Tomás Cohen Arazi 2022-12-02 13:44:31 UTC
Aren't the parameter confusing but the outcome correct?
Comment 2 Jonathan Druart 2022-12-02 13:45:52 UTC
This one could be considered critical actually.
Comment 3 Jonathan Druart 2022-12-02 13:46:37 UTC
Created attachment 144400 [details] [review]
Bug 32394: Restore long tasks queue

We are sending "queue" as parameter, but "job_queue" is retrieved in
Koha::BackgroundJob->enqueue...

Test plan:
Read code and confirm the above.
More fun for test plan will be found on bug 32393
Comment 4 Jonathan Druart 2022-12-02 13:46:53 UTC
(In reply to Tomás Cohen Arazi from comment #1)
> Aren't the parameter confusing but the outcome correct?

All jobs are using the default queue.
Comment 5 David Cook 2022-12-04 22:46:25 UTC
(In reply to Jonathan Druart from comment #4)
> All jobs are using the default queue.

I thought ES indexing was supposed to use the long one?
Comment 6 David Cook 2022-12-04 22:47:16 UTC
(In reply to David Cook from comment #5)
> (In reply to Jonathan Druart from comment #4)
> > All jobs are using the default queue.
> 
> I thought ES indexing was supposed to use the long one?

Maybe I had that backwards. I see now on the patch...
Comment 7 Tomás Cohen Arazi 2022-12-06 12:21:43 UTC
Created attachment 144444 [details] [review]
Bug 32394: Restore long tasks queue

We are sending "queue" as parameter, but "job_queue" is retrieved in
Koha::BackgroundJob->enqueue...

Test plan:
Read code and confirm the above.
More fun for test plan will be found on bug 32393

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Marcel de Rooy 2022-12-09 10:41:40 UTC
prove t/db_dependent/Koha/BackgroundJob*
t/db_dependent/Koha/BackgroundJob.t ...................... ok
t/db_dependent/Koha/BackgroundJobs/BatchDeleteBiblio.t ... ok
t/db_dependent/Koha/BackgroundJobs/BatchDeleteItem.t ..... ok
t/db_dependent/Koha/BackgroundJobs/BatchUpdateBiblio.t ... ok
t/db_dependent/Koha/BackgroundJobs/StageMARCForImport.t .. ok
t/db_dependent/Koha/BackgroundJobs.t ..................... ok
All tests successful.
Comment 9 Marcel de Rooy 2022-12-09 10:43:30 UTC
Created attachment 144524 [details] [review]
Bug 32394: Restore long tasks queue

We are sending "queue" as parameter, but "job_queue" is retrieved in
Koha::BackgroundJob->enqueue...

Test plan:
Read code and confirm the above.
More fun for test plan will be found on bug 32393

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2022-12-09 10:44:34 UTC
Warning for the RM: trivial fix but module changes without unit tests ;)
This got perhaps unnoticed since we didnt test ?
Comment 11 Jonathan Druart 2022-12-09 19:06:09 UTC
I will provide tests.
Comment 12 Tomás Cohen Arazi 2022-12-09 21:27:52 UTC
(In reply to Marcel de Rooy from comment #10)
> Warning for the RM: trivial fix but module changes without unit tests ;)
> This got perhaps unnoticed since we didnt test ?

Thanks. My plan was to provide the tests on pushing. But better fail it
Comment 13 Jonathan Druart 2022-12-14 08:01:53 UTC
Created attachment 144567 [details] [review]
Bug 32394: Add tests
Comment 14 Jonathan Druart 2022-12-14 08:03:48 UTC
Created attachment 144568 [details] [review]
Bug 32394: Add tests
Comment 15 Tomás Cohen Arazi 2022-12-14 17:35:23 UTC
Created attachment 144585 [details] [review]
Bug 32394: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2022-12-14 17:35:28 UTC
Created attachment 144586 [details] [review]
Bug 32394: Restore long tasks queue

We are sending "queue" as parameter, but "job_queue" is retrieved in
Koha::BackgroundJob->enqueue...

Test plan:
Read code and confirm the above.
More fun for test plan will be found on bug 32393

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2022-12-14 17:35:36 UTC
Created attachment 144587 [details] [review]
Bug 32394: Add tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2022-12-14 17:54:37 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 19 Martin Renvoize 2022-12-15 13:57:57 UTC
Nice work everyone!

Pushed to 22.11.x for the next release
Comment 20 Tomás Cohen Arazi 2022-12-16 12:01:37 UTC
Created attachment 144670 [details] [review]
Bug 32394: (follow-up) Add param for Koha::BackgroundJob::StageMARCForImport->enqueue

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Martin Renvoize 2022-12-18 08:22:18 UTC
Nice work everyone!

Pushed to 22.11.x for the next release
Comment 22 Lucas Gass 2022-12-21 18:49:15 UTC
This doesn't apply cleanly to 22.05.x, if needed please rebase
Comment 23 Jonathan Druart 2023-02-01 16:13:08 UTC
Created attachment 145964 [details] [review]
Bug 32394: [22.05.x] Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 24 Jonathan Druart 2023-02-01 16:13:14 UTC
Created attachment 145965 [details] [review]
Bug 32394: [22.05.x] Restore long tasks queue

We are sending "queue" as parameter, but "job_queue" is retrieved in
Koha::BackgroundJob->enqueue...

Test plan:
Read code and confirm the above.
More fun for test plan will be found on bug 32393

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 25 Jonathan Druart 2023-02-01 16:13:19 UTC
Created attachment 145966 [details] [review]
Bug 32394: [22.05.x] Add tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Jonathan Druart 2023-02-01 16:13:33 UTC
I have not tested the rebase!
Comment 27 Lucas Gass 2023-02-01 16:35:34 UTC
(In reply to Jonathan Druart from comment #26)
> I have not tested the rebase!

Thanks Jonathan, it seems to work but I'd like one more sign-off besides my own before I push to 22.05.x
Comment 28 Lucas Gass 2023-02-01 18:16:14 UTC
(In reply to Jonathan Druart from comment #26)
> I have not tested the rebase!

Tomas and I tested the rebase together and everything is good.
Comment 29 Lucas Gass 2023-02-01 18:21:02 UTC
Backported to 22.05.x for upcoming 22.05.10
Comment 30 Arthur Suzuki 2023-02-17 09:31:48 UTC
depends on 27783, can't applied on 21.11.x