feat: return job IDs from Worker::perform_later() for status tracking#1624
feat: return job IDs from Worker::perform_later() for status tracking#1624NewtTheWolf wants to merge 19 commits intoloco-rs:masterfrom
Conversation
|
i did not changed anything in the files who failed sanity 🤔 |
|
thanks @NewtTheWolf! . |
|
yes, i can take a look this week |
|
Hey @NewtTheWolf, Steps to Reproduce
Expected Actual i got: job: None |
|
hey @kaplanelad thats an expected result a Job ID ONLY gets returned if the Worker Supports it, if it does not Support JobId's it returns None I saw you used Async Worker and Async Worker dont Return or even Generate Job ID's if i'm right? i used this comment as Inspiration #1039 (comment)
|
The intention is that we can switch queues behind the scenes and the app continues to work. Adding an optional Job ID could break logic when switching between different job queues. Wouldn’t it make sense to generate a Job ID for all queues to avoid this issue? |
|
it would make sense absoloutly. i generally like the idea of #1624 (comment) sorry i really didnt had time for this, my goal is to update this pr this week! |
|
@NewtTheWolf do you need any help getting this across? This is quite a helpful feature that I'd love to get in. Let me know if you'd like any assistance. |
|
hey @mccormickt, sorry had a loot going on in my live lately. But your Message was a small boost to my Motivation! i will start now! first of all making the Tokio Worker to return an ID so it is no longer Optional and then making the fn |
|
@NewtTheWolf I'd also appreciate this feature :) |
omg thats completly my fault. i thought it is finished, i just realize now that one CI has an error.... i will fix it tomorrow (UTC+1) |
the best timezone 😂 No worries, looking forward to using it. |
…s#1624) - Queue::enqueue() now returns Result<Option<String>> with job ID - BackgroundWorker::perform_later() returns job ID for tracking - Queue::get_jobs made public for external job querying - Exposes tokio task ID in BackgroundAsync worker mode
|
FINALLY |
There was a problem hiding this comment.
LGTM! Thanks for hanging in there and getting this across the line.
cc @kaplanelad
|
Thanks @NewtTheWolf A few things I'd like us to address before merging: Inconsistent ID semantics across worker modesThe main goal here is status tracking, but the three modes return very different kinds of IDs:
Could we at least generate a ULID for Error on missing provider (mod.rs lines 651-654)Previously, if
|
|
i will take a look into it.
Yes! but i think we should do an internal mapping from Ulid to TaskId in case we have a usecase for it in the future like with a Map (or smth similar)
i think so yes! it is on my todo board! |
|
Hey @kaplanelad, I addressed your points: Inconsistent ID semantics: Error on missing provider: |
Summary
This PR implements job ID returns from
Worker::perform_later()to enable job status tracking, addressing #1623.Changes
Queue::enqueue()to returnResult<Option<String>>instead ofResult<()>BackgroundWorker::perform_later()to returnResult<Option<String>>instead ofResult<()>Some(job_id)when using BackgroundQueue mode with a configured providerNonefor ForegroundBlocking, BackgroundAsync modes, or BackgroundQueue without a providerTesting
enqueuetests to verify job ID returnsqueue_enqueue_returns_job_idto verify behavior for different queue modescargo test --features bg_redis,bg_pg,bg_sqltDocumentation
Queue::enqueue()methoddocs-site/content/docs/processing/workers.mdNotes for Reviewers
Implements #1623