dereuromark / cakephp-queue

Queue plugin for CakePHP - simple, pure PHP and without dependencies.
MIT License
306 stars 137 forks source link

createJob() requires requested job type (Task file) to be present when created #288

Closed ryanolton closed 3 years ago

ryanolton commented 3 years ago

As I updated to version 6.x my createJob() calls began failing on my test servers. I see in the code now that when creating a job that jobType() is called which resolves that the job type (Task file) indicated in createJob() is actually present within the code.

No job type can be resolved for [Job Type]

I have a number of servers that create jobs via createJob() and then a separate group of servers actually processing the jobs. I do not have the Task code on all servers because most of these servers never actually process jobs, they merely create them.

Would it be possible via a configuration setting to be able to "not resolve" job types so that on the servers merely creating jobs that the job type is assumed to be "resolved" (at my peril)?

$config['Queue']['resolvejobtypes'] = false;

protected function jobType(string $jobType): string {
  if (Configure::read('Queue.resolvejobtypes') === false) {
      return $jobType;
  }

  if ($this->taskFinder === null) {
      $this->taskFinder = new TaskFinder();
  }

  return $this->taskFinder->resolve($jobType);
}

Or, instead, add this option to the createJob() $config:

public function createJob(string $jobType, ?array $data = null, array $config = []) {
    $queuedJob = [
        'job_task' => !empty($config['resolvejobtype']) && $config['resolvejobtype'] === false ? $jobType : $this->jobType($jobType),
        'data' => is_array($data) ? serialize($data) : null,
        'job_group' => !empty($config['group']) ? $config['group'] : null,
        'notbefore' => !empty($config['notBefore']) ? $this->getDateTime($config['notBefore']) : null,
    ] + $config;

    $queuedJob = $this->newEntity($queuedJob);

    return $this->saveOrFail($queuedJob);
}

I'm not sure what's best here, and perhaps my situation is too unique?

dereuromark commented 3 years ago

You have quite a unique Szenario as the Code base should be the same, and as such the tasks available.

But the resolvement Was mainly added to catch typos earlier than in production. We could remove them for Direct string usage, as here we dont technically need the lookup.

ryanolton commented 3 years ago

You have quite a unique Szenario as the Code base should be the same, and as such the tasks available.

But the resolvement Was mainly added to catch typos earlier than in production. We could remove them for Direct string usage, as here we dont technically need the lookup.

I think removing the check for direct string usage would work for me. Do you need a PR?

dereuromark commented 3 years ago

The problem is that this would also remove the plugin deprecation help when using non plugin call. In end it sounds rather like some config is needed here if we want to allow this.

What is the reason you are not checking out the tasks on the other systems? The disk space is free, so for me this still sounds a bit like over-optimisation of sorts. As this would then just resolve the problem already by design.

ryanolton commented 3 years ago

The reason is simply multiple code bases: we split the code base into essentially the "user" side and an "admin" side. We further split the queued task side of things into a dedicated code base that handles queue processing only. Some of our queued jobs have actually have been migrated to Go and Rust to improve speed and efficiency, and in these cases there are no longer any corresponding PHP code task present. I suppose we could insert stub files in order to satisfy the plugin's check, that may be the best solution at this point. Alternatively, we can easily remain on version 5.x for now and reevaluate our situation going forward.

I admit that all of this is beyond the typical use case. We have been using this plugin since the start of our project and have simply grown to where it might be time for something custom made for our current/future needs.

I'll leave it to you whether a change to accommodate this; please do what you feel is best for the intended audience. Regardless of your decision, your plugin has been very helpful, so thank you for your hard work!

dereuromark commented 3 years ago

What do you think of https://github.com/dereuromark/cakephp-queue/pull/289 ?

ryanolton commented 3 years ago

What do you think of #289 ?

289 would certainly do the trick, thank you!

dereuromark commented 3 years ago

Maybe you can PR a test case or alike? I think then we can move forward.