Skip to content

feat(job): implement moveToWaitingChildren for sandboxed jobs #3231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shichongrui
Copy link

Why

moveToWaitingChildren is not implemented on sandboxed jobs. The docs don't explain this and from what I could see there was no reason for it not to be other than that it just wasn't implemented there.

How

I followed the same approach that the moveToDelayed function uses in a sandboxed job to send commands back to the parent process to actually execute the functionality.

Additional Notes (Optional)

I did not see any tests for moveToDelayed that tested from within a sandboxed process so I wasn't sure if there was somewhere to add tests for this functionality.

I've done a yarn pack and then untarred it in a local bullmq project and confirmed that it was working though.

Fixes #3230

@manast manast requested review from roggervalf and Copilot April 21, 2025 22:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the moveToWaitingChildren functionality for sandboxed jobs by adding a new enum value and handling the command in both the sandbox and child-processor modules.

  • Added MoveToWaitingChildren to the ParentCommand enum.
  • Updated sandbox.ts and child-processor.ts to proxy moveToWaitingChildren commands.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/enums/parent-command.ts Added the MoveToWaitingChildren enum
src/classes/sandbox.ts Added a case to handle MoveToWaitingChildren commands in the sandbox
src/classes/child-processor.ts Introduced a proxy function to forward moveToWaitingChildren commands
Comments suppressed due to low confidence (2)

src/classes/sandbox.ts:55

  • Add tests covering the MoveToWaitingChildren command handling in sandbox mode to ensure its correct behavior.
case ParentCommand.MoveToWaitingChildren:

src/classes/child-processor.ts:161

  • Introduce unit tests for the moveToWaitingChildren proxy function to validate its command forwarding functionality.
moveToWaitingChildren: async (token?: string) => {

@manast
Copy link
Contributor

manast commented Apr 21, 2025

Not having a test for moveToDelayed was definitely an overlook from out part, we will fix it asap.

@manast
Copy link
Contributor

manast commented Apr 24, 2025

I actually checked in the codebase and there is indeed a test for moveToDelayed, you can use it for inspiration for your moveToWaitingChildren: https://github.com/taskforcesh/bullmq/blob/master/tests/test_sandboxed_process.ts#L892

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.

Implementing moveToWaitingChildren in SandboxedJobs
2 participants