Feature: pass Job by id

[frederikheld]

Problem

IMHO the whole "pass Job by path" approach is not a good idea. It works well enough if job runner, the code that enqueues the job and the definition of the job are in the same package. But the beauty of a system like sidequest is that you can separate the job runner from the API that creates them and deploy both independently.

It was very hard setting up the job runner and the API that creates the jobs in different packages of my monorepo. Ending up with the exact same paths would have forced a file structure that I did not want. So the fix was symlinking the build results of a separate third package that defines the jobs into the two packages that use those definitions. Eventually, this worked. And lucky me, Git can store symlinks.

But then I dockerized these packages and learned that Docker doesn't copy symlinks. So now I have to look for another workaround.

The path approach requires a lot of workarounds and debugging. And looking at my database, I see job paths like this: ../../../../../../../../../../sidequest-jobs/SomeJob.ts. It's hard to debug such things, it almost feels like callback hell back in the time.

Add Docker on top of that (which is hard to debug in itself) and you waste a lot of time poking around to fix "cannot find module" errors thrown by the job runner.

Proposed Solution

Wouldn't it be possible to have unique id's per job and then store the job id instead of the path in the database?

At start, the job runner could check for uniqueness and then create an index of all jobs, which would allow for fast access as well.

This approach would make it way easier to use sidequest in a more flexible architecture.

[merencia]

hello @frederikheld

I understand, depending on how the application is designed we’ll need some hacks to make it work.

Node.js unfortunately doesn’t share the loaded context with child processes or worker threads. So when we have a job instance we can’t pass the instance to the worker, it’s serialized and only the plain object is received inside the worker.

So let’s say your job does something like this:

import { Job } from 'sidequest';
import coolLib from 'cool-lib';

export class MyJob extends Job {
  run(){
    return coolLib();
 }
}

We need the path to load the job file and by consequence its dependencies. When we recreate the job instance by calling new MyJob inside the worker everything works, including the external lib call.

At the end of the day to have this automatic context loaded inside an isolated Node process we must import the job, and to do so we need the path.

About your suggestion, I’m not sure I got how it would work. Do you see a way to load all the job’s dependencies into an isolated process by id only?

[frederikheld]

I think I understand the issue here. But you don't need to pass the path in order to load the job from the path. You could load all paths at start, grep out their uid and put them in an object that maps the uid to the path. That way you could pass the uid through the database and then load the local path for that id. Of course you would need to make sure that there are no duplicates, which adds complexity.

A different way to improve the situation with the current approach would be to allow setting a base path. So you would just write the path relative to the jobs package to the database and each package that needs to access that job (runner and api in my case) sets their own base path that points to the jobs package. That way, the path in the database would be the same for all, no matter how the repo is structured.

Having a path relative to the root of the jobs package would also allow me to just install that package as a local dependency into the other packages. That is how I would usually deal with common packages and it feels like the more JS way than dealing with actual file system paths.

[GiovaniGuizzo]

Hey @frederikheld Let me see if I understand you.

But you don't need to pass the path in order to load the job from the path

How do you load a job from a path without a path? Even if you pass the type, like we do now, we still need to map the job's path so that any other machine using it will be able to find it.

You could load all paths at start, grep out their uid and put them in an object that maps the uid to the path.

What do you mean by "load all paths at the start"? Load from where? How?

A different way to improve the situation with the current approach would be to allow setting a base path. So you would just write the path relative to the jobs package to the database and each package that needs to access that job (runner and api in my case) sets their own base path that points to the jobs package. That way, the path in the database would be the same for all, no matter how the repo is structured.

So, when starting Sidequest, you would pass a "basePath"? In this sense, on your API you would pass the jobs package path as basepath? How is this different from passing a relative path to Sidequest's? The solution right now is to use Sidequest's job class' path as a basePath to everything. So if all your packages share the same node_modules dir, you already have the basePath set.

Having a path relative to the root of the jobs package would also allow me to just install that package as a local dependency into the other packages. That is how I would usually deal with common packages and it feels like the more JS way than dealing with actual file system paths.

If you install your package A as a dependency to package B, and package A uses Sidequest, you will still have Sidequest in your node_modules and the basePath will be resolved anyway.

Sorry, but I am not understanding your problem here. Can you give an example?

[frederikheld]

Here's a minimal example: https://github.com/frederikheld/sidequest-example

The sidequest-jobs package is installed as local dependency into api and job-runner. I can import the Job in api like this:

import { ExampleJob } from 'sidequest-jobs'

I can enqueue it and and it will show up in the database. But then the job runner won't find the script, although the relative path to ExampleJob.js is the same for job-runner as well as for api. So it doesn't actually have to do with the different architecture in my actual project.

The path in the database is ../../../../../dist/jobs/ExampleJob.js which is some levels deeper than the actual relative path. I don't understand how Sidequest comes up with the relative path, but it is wrong.

In my actual project i "fixed" this by symlinking sidequest-jobs/dist to a path within job-runner that matches the relative path. But this is hacky.

[frederikheld]

About your other questions:

So, when starting Sidequest, you would pass a "basePath"? In this sense, on your API you would pass the jobs package path as basepath? How is this different from passing a relative path to Sidequest's?

The difference is, that I can use different base paths in api and job-runner, which decouples those packages. Right now, Sidequest assumes that everything happens in the same package and I still have to make sure that the enqueue and the runner have the same relative path to the job definition. But my architecture isn't structured like this. So I have to break my architecture pattern to make it work.

The whole base path thing gets a lot easier if we let npm handle it. Because then the base path is just some path in node_modules which will always be the same for any package that installs the sidequest-jobs package.

The following answers refer to my proposed solution which is different from the current one: I propose to use a unique id to refer to a job instead of its file path. If we get the paths approach to be more flexible, this won't be necessary because it would be more complicated to implement. In the end it doesn't matter how jobs are being referred to, if it just works without jumping hoops. But for the sake of discussion ideas, I'll share my thinking. Just don't mix the approaches up!

What do you mean by "load all paths at the start"? Load from where? How?

How do you load a job from a path without a path?

Vite has a glob import feature. I'm thinking about something similar. The job runner knows (through configuration) the path to the directory that holds the job definitions. It can now glob import all files that match the pattern.

The constructor of the Job class could auto-generate a uid which is stored as a reference in the DB when the job is enqueued.

When the job runner is started, it glob imports all jobs, extracts the Job.uids from all imports and then builds a map from it:

const jobsMap: {
   '3eab85a2-380d-49e9-814c-a4a0efddd951': FooJob,
   '31274d05-34cf-4ba5-9e9b-3149c344bcc5': BarJob
}

When it executes a job, the job runner could then use that Job.uid to find the right import from the globs.

This is a more complicated approach, but it would remove the dependency from exact file paths.

I don't know if becoming more flexible is the direction you intend to go with Sidequest (given that it is intended to be a simple alternative to the big job runners), but IMHO the unique selling point of a job runner is to offload the heavy lifting to dedicates processes that can be scaled independently. So it makes sense to deploy them independently from the rest of the software. It would be great if this could be done following the established NodeJS patterns, namely package imports.

[GiovaniGuizzo]

@frederikheld I think I understood what you want to do. We will work on a solution where you can pass a custom script path to the JobBuilder. This way you can simply bypass the path resolution and provide the path you need.
You can then do this job ID thing internally in your application.

I feel like this ID solution would add so much more complexity to Sidequest that it would defeat the purpose of the tool. It's supposed to be minimal config, minimal management, etc.

This new feature (coming soon) will solve a few problems in other issues as well. I believe it will solve yours. It's gonna be something like this:

Sidequest.build(MyJob).script("custom/path/to/my/job.js").enqueue()

[iffa]

Chiming in to say that I also liked the original proposal as that would then be quite straightforward to setup for us as consumers of this lib. And on a related note I see it currently just refactoring your codebase means you'll encounter a job path footgun.

[frederikheld]

Sidequest.build(MyJob).script("custom/path/to/my/job.js").enqueue()

This isn't the solution I'm looking for. From a DX perspective, I don't want to deal with those paths at all. No other package in the npm ecosystem asks me to deal with paths of compiled scripts. Sidequest should handle that logic internally.

I'm fine with whatever Sidequest is using to refer to a Job. Path, Id, nickname, I don't care. Because it should happen internally.

I like the current API to enqueue Jobs and I don't really understand why Jobs are so dependent on the script path. Why can't the job runner find the right script to execute? I fully agree with @iffa: using paths is a footgun, because it makes my code super inflexible.

My proposal to import all scripts and then filter by uid is just one way to do it. It doesn't matter which solution you choose, but I don't think that it would be a good idea to change the API and offload the handling of this logic to the user.

[GiovaniGuizzo]

I see what you mean. I will talk to @merencia and see if we can find a solution.
The problem with "importing all jobs" is that we will need to analyze scripts at runtime and check which ones extend the Job class. Just adding a glob wouldn't solve the issue, as a glob would include all files. Moreover we need to know which class to import, so if a job script exports 2 jobs, we need to map that as well.

[frederikheld]

Yes, sure, it will add some overhead at startup. It might take some milliseconds to analyze the jobs and build the index. But as soon as that is done, this solution is as performant as the current solution.

Reading all files once at startup should even be more performant than loading the script from the file system every time you want to execute it. Object property lookup is O(1) source. So it's the same as accessing files, but from RAM, which makes it faster.

The only performance bottleneck I can think of is when users add millions of Job definitions that all have to be indexed and kept in RAM. IDK what the real numbers are, in my project I have about 20 Job definitions and even with future plans in mind, the number will stay well below 100.

[merencia]

@frederikheld in your suggestion how would you load the job file inside the worker (a new node process) ? Didn't get that.

[frederikheld]

I don't know the internal structure of Sidequest well enough. But I'm beginning to understand that the path approach comes from the fact that the work is done in worker threads. Is this correct?

[merencia]

yes, it's a working thread, so it's kinda a new process, with a new v8 and event loop... So, we get the path to do a dynamic import and load everything that is imported on the job to make it work.

[frederikheld]

A)

Data passed to the worker is stringified. You can just pass the function to the worker and re-create the function inside the worker using new Function() or eval(). new Function() might be the safer choice.

B)

Pass the script as you are used to but add some logic to the runner that finds the correct path to pass. In this case you wouldn't do a glob import but you could just read the jobs directory with fs instead and then do the parsing for uids on the raw files. Then you don't map the uid to a function but to a script path.

From a developers PoV, the whole thing should work with normal module imports as it currently does. But that doesn't mean that you can't let the runner parse the files instead of doing imports. If that part is hidden from the developer, it's a valid approach IMHO.