Skip to content
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

[Feature Request] Support separating bundling and executing of workers #310

Closed
schickling opened this issue Oct 19, 2021 · 6 comments · Fixed by #336
Closed

[Feature Request] Support separating bundling and executing of workers #310

schickling opened this issue Oct 19, 2021 · 6 comments · Fixed by #336
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@schickling
Copy link

Currently the worker entrypoint file(s) (e.g. src/worker.ts) are doing both bundling and execution.

CleanShot 2021-10-19 at 16 12 20

For my application, I'd like to take the responsibility of the bundling (and e.g. use something like esbuild instead of Webpack) and have my worker entrypoint (e.g. invoked by nodemon) only trigger the actual code execution and not any kind of building/bundling when starting.

@schickling schickling added the enhancement New feature or request label Oct 19, 2021
@bergundy
Copy link
Member

@schickling it'd take some work to support bundling with anything other than webpack from my experience.
The bundling process isn't that straight forward, user code needs to be lazily loaded so the runtime can be properly prepared for workflow execution.
It should be straight forward to save the bundle to disk and load it on worker startup for cases like production deployment.

@bergundy bergundy self-assigned this Oct 19, 2021
@bergundy bergundy added this to the beta milestone Oct 20, 2021
@bergundy
Copy link
Member

@lorensr @schickling @sw-yx I'd like some feedback on the public API for this.

In order to support this we'd need to export the WorkflowCodeBundler class added in #317 for users to be able to create the bundle.
In addition we need to understand how to pass this into the WorkerOptions, a few options come to mind.

  1. Remove the workflowsPath and nodeModulesPaths options in favor of a new workflowCodeBundle option
  • Pro: much more explicit about what's going on
  • Con: More verbose samples
  1. Add the workflowCodeBundle option to the existing options.

workflowCodeBundle could either be a path (string), the bundle itself (string / buffer / stream), I'm not sure what would be most convenient.
I'm leaning towards the string because it's the most flexible option.

Example:

const workflowCodeBundle = await WorkflowCodeBundler({ workflowsPath });
const worker = await Worker.create({ workflowCodeBundle, ... });

@lorensr
Copy link
Contributor

lorensr commented Oct 20, 2021

I'm leaning towards the string because it's the most flexible option.

By string do you mean path?

Awaiting a SentenceCase function feels weird to me. SentenceCase makes me want to do new WorkflowCodeBundler() or WorkflowCodeBundler.bundle(). What about:

// built-in bundling
const workflowCodeBundle = await bundleWorkflowCode({ workflowsPath });
const worker = await Worker.create({ workflowCodeBundle, ... });

// dev doing their own bundling:
const worker = await Worker.create({ workflowCodeBundle: './bundle.js', ... });

That thing is happening where the word "bundle" is starting to sound really strange to me 😆

@bergundy
Copy link
Member

So the return value of bundleWorkflowCode is a string which means that we can't use pass a path using the same parameter name.
I usually like to call path params what they are for clarity, e.g. workflowCodeBundlePath.
Another option is to have workflowCodeBundle param type: { path: string } | { code: string } and have bundleWorkflowCode return { code: string }.

@lorensr
Copy link
Contributor

lorensr commented Oct 25, 2021

We could have both bundleWorkflowCode and workflowCodeBundlePath:

// built-in bundling
const workflowCodeBundle = await bundleWorkflowCode({ workflowsPath });
const worker = await Worker.create({ workflowCodeBundle, ... });

// dev doing their own bundling:
const worker = await Worker.create({ workflowCodeBundle: 'js code', ... });
const worker = await Worker.create({ workflowCodeBundlePath: './bundle.js', ... });

@bergundy
Copy link
Member

IDK, I kind of like the { path: string } | { code: string } solution.
Removes some of the clutter from the already overwhelming WorkerOptions interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants