Bug 32395

Summary: Why do we send jobs params to the broker
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: dcook, m.de.rooy, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 32305, 22417, 32393    
Bug Blocks: 35092    
Attachments: Bug 32395: Do not send job's parameters to the broker

Description Jonathan Druart 2022-12-02 14:07:43 UTC
We are:
- Building the parameters
- Insert a job and its parameters in DB
- Send a message to the broken with the parameters
- Retrieve the message from the worker
- Process the job using the parameter from the broker

Why don't we simply pass the job_id to the broker, then retrieve the parameters from the DB in the worker?
Comment 1 Jonathan Druart 2022-12-02 14:12:21 UTC
We would have avoided all the JSON encoding/decoding stuffs we had recently, and bug 32393.
Comment 2 David Cook 2022-12-04 22:41:11 UTC
(In reply to Jonathan Druart from comment #0)
> Why don't we simply pass the job_id to the broker, then retrieve the
> parameters from the DB in the worker?

In theory, a loosely coupled message consumer might not have database access to the app, so you'd want to encapsulate everything it needs in the message.

However, the system designed for Koha is tightly coupled, so yeah... we could just pass the job_id in the message and have the worker fetch the details. It adds more database I/O, but it probably wouldn't be significant. In some cases, it could actually be more efficient. 

I don't know what the best practice is, and I imagine we should just do whatever makes sense for us.
Comment 3 David Cook 2022-12-04 22:44:48 UTC
In the context of bug 32393 I see your point even more clearly.

Since we're tracking jobs in the database and not doing a fire and forget pattern, it probably does make sense to just pass the job_id, because the message really is  just "oh hey, I've got a job. Here's it's ID. Grab it and do stuff with it."
Comment 4 Jonathan Druart 2022-12-06 09:31:05 UTC
Created attachment 144434 [details] [review]
Bug 32395: Do not send job's parameters to the broker
Comment 5 Jonathan Druart 2022-12-06 09:33:00 UTC
The logic is here, but I don't think this is enough. It must be more robust and clear. Do we allow other parameters than job_id? Should we simply pass the id as a string an completely remove JSON?

I don't think so, we should keep flexibility and keep in mind we could have jobs that don't go through the DB.
Comment 6 Tomás Cohen Arazi 2023-01-27 10:53:36 UTC
(In reply to Jonathan Druart from comment #5)
> The logic is here, but I don't think this is enough. It must be more robust
> and clear. Do we allow other parameters than job_id? Should we simply pass
> the id as a string an completely remove JSON?
> 
> I don't think so, we should keep flexibility and keep in mind we could have
> jobs that don't go through the DB.

Let's pass a JSON with the job_id.