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

[question] workerData parameter shape #54

Open
naz opened this issue Nov 24, 2020 · 2 comments
Open

[question] workerData parameter shape #54

naz opened this issue Nov 24, 2020 · 2 comments

Comments

@naz
Copy link
Member

naz commented Nov 24, 2020

The problem I'm facing is a confusing (from job developer perspective) use of the data (workerData) passed into the worker. After looking closer into how workerData parameter is constructed, I have a proposal on how to structure it so it's more intuitive for the job developer.

Let's consider an example bree job initialization with a value passed in:

// bree client configuring a job with passed in data

const job = {
    cron:'0/5 * * * * *',
    name:'job-with-data',
    path:'/path/to/job-using-data.js',
    worker:{
      workerData: 42
    }
};

bree.add(job);
bree.start('job-with-data');
// job-using-data.js file content

const {workerData} = require('worker_threads');

(async () => {
    const data = workerData.job.worker.workerData;
    console.log(data); // prints '42'
})();

To get access to the data inside of the job file job developer has have knowledge about quite complicated bree's internal object structure workerData.job.worker.workerData. This structure doesn't match 1:1 to the parameter passed during the job initialization. As a developer writing a job, I would expect to have access to same data structure as if I initialized a job through native Worker constructor:

new Worker('/path/to/job-using-data.js', {workerData: 42});
// job-using-data.js file content

const {workerData} = require('worker_threads');

(async () => {
    const data = workerData;
    console.log(data); // prints '42'
})();

I don't see a good reason why bree job would need to have all the meta information about itself accessible from within the script. I think it's unnecessary for the job to know about it's name, interval or any other bree's internal configurations. It should be up to bree's client to use these configurations and the job itself should execute in self contained manner, with as little knowledge as possible. @niftylettuce what are your thoughts? Could you please clarify the usecase for passing in job metadata into workerData parameter?

Proposed change

I think initializing worker in following way would make reading workerData inside of jobs most intuitive:

const workerData = job.worker && job.worker.workerData ? job.worker.workerData : undefined;
const object = {
  ...(this.config.worker ? this.config.worker : {}),
  ...(job.worker ? job.worker : {}),
  workerData
};

this.workers[name] = new threads.Worker(job.path, object);
@shadowgate15
Copy link
Member

I'm pretty sure this is the way workerData is working now...am I incorrect?

@nickperkinslondon
Copy link

No, this is till an issue I think, I just found this issue, and it helped me add this to my job code:

{ Worker, isMainThread, workerData } = require('worker_threads');
my_data = workerData.job.worker.workerData;

...to avoid a bunch of strange data ( bree internal stuff? )
Now I just get the data I was expecting.

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

No branches or pull requests

3 participants