Skip to content

fix: Do not process job; instead re enqueue if shutdown was requested#281

Closed
vikigenius wants to merge 1 commit into
tobymao:mainfrom
vikigenius:main
Closed

fix: Do not process job; instead re enqueue if shutdown was requested#281
vikigenius wants to merge 1 commit into
tobymao:mainfrom
vikigenius:main

Conversation

@vikigenius

@vikigenius vikigenius commented Nov 27, 2025

Copy link
Copy Markdown
Contributor

Fixes #280

@tobymao

tobymao commented Nov 27, 2025

Copy link
Copy Markdown
Owner

it's unclear exactly what problem this is solving. can you write a test?

@vikigenius

Copy link
Copy Markdown
Contributor Author

The idea is that if you were waiting a while for dequeue, a signal could have arrived in the meantime, but if you go ahead and start the job without checking you just started a job even though shutdown was requested.

I will take a look at the tests and write one to showcase the behavior.

@tobymao

tobymao commented Nov 27, 2025

Copy link
Copy Markdown
Owner

i don't think you should re-enque it, it should probably just fail and let the retry handle it

@vikigenius

Copy link
Copy Markdown
Contributor Author

You are right, I don't think it's good to enqueue again and just introduces potential complexity. If we are letting it fail, we don't need to do anything, it will auto fail after timeout anyway I will close the PR

@vikigenius vikigenius closed this Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Why not re-qneueu/put back jobs if event set

2 participants