Chainedtask order

[jvancoillie]

How does the scheduling of a chainedtask work ?
According to the order defined in the yaml configuration ?
Can we use the priority configuration entry to order the chained tasks ?

[Guikingone]

Hi @jvancoillie 👋🏻

That's an ... interesting point that you're mentioning here 😄

At the beginning of the development of this bundle, the feature was present then ... I remove it for some random reasons, by default, tasks are executed in the order defined in the configuration file (no matter the format).

We can sort the tasks using priority but the feature is not available for now, maybe you want to contribute on it? 🙂

[jvancoillie]

Of course, I volunteer to contribute. I'm afraid I won't be up to the task, but I want to give it a try. I'll see what I can do on my end about this. It is a good exercise to better understand the functioning and the philosophy of this bundle.

[jvancoillie]

Hi @Guikingone,

I have just set up an environment to study the bundle and see how we could deal with the notion of priority.

I realize that the order of the configuration of the task is not that which one finds at the execution, therefore it seems important to me to introduce the notion of priority.

my yaml configuration :

scheduler_bundle:
  transport:
#    dsn: 'filesystem://first_in_first_out'
    dsn: 'doctrine://default?execution_mode=first_in_first_out'
  tasks:
    world:
      type: 'command'
      command: 'app:world'
      expression: '* * * * *'
      description: 'world command'
    hello:
      type: 'command'
      command: 'app:hello'
      expression: '* * * * *'
      description: 'hello command'

the bin/console scheduler:list

the bin/console scheduler:consume

we can see that the order of execution is not the same as the configuration. in both transports filesystem and doctrine.

How do you see priority management ?
do we need to add a priority policy ?

[Guikingone]

However, I thought that the first_in_first_out policy would be configuration-based.

it is, it's based on the execution_mode option passed in the transport DSN 🙂

[jvancoillie]

🤔 This is not what I find with the above configuration, I expected to have the world command before the hello command

[Guikingone]

Yes, I agree, strange behaviour here 🤔

[Guikingone]

I've opened an issue related to the sorting problem that you're facing.

[Guikingone]

A fix has been merged for the FIFO sort 🙂

[jvancoillie]

Is a simple sorting on the priority of the tasks sufficient or should there be an execution mode for the 'chained' type that could orchestrate the list.

if a simple sort is sufficient, is it preferable to sort the tasks at the build of the chainedTask

    /**
     * {@inheritdoc}
     */
    public function build(PropertyAccessorInterface $propertyAccessor, array $options = []): TaskInterface
    {
        $tasks = array_map(function (array $task) use ($propertyAccessor): TaskInterface {
            foreach ($this->builders as $builder) {
                if (!$builder->support($task['type'])) {
                    continue;
                }

                return $builder->build($propertyAccessor, $task);
            }

            throw new InvalidArgumentException('The given task cannot be created as no related builder can be found');
        }, $options['tasks']);

        uasort($tasks, fn (TaskInterface $task, TaskInterface $nextTask): int => $task->getPriority() <=> $nextTask->getPriority());

        $chainedTask = new ChainedTask($options['name'], ...$tasks);

        unset($options['tasks']);

        return $this->handleTaskAttributes($chainedTask, $options, $propertyAccessor);
    }

or at each call of the getTasks() method

    /**
    * @return TaskInterface[]
    */
   public function getTasks(): array
   {
       uasort($this->options['tasks'], fn (TaskInterface $task, TaskInterface $nextTask): int => $task->getPriority() <=> $nextTask->getPriority());

       return $this->options['tasks'];
   }

[Guikingone]

IMHO, sorting tasks at the building phase is enough, it depends if we want to allow sorting on criteria like the policy orchestrator does 🤔

[Guikingone]

I'm starting to think that maybe, we can inject the SchedulePolicyOrchestrator in the ChainedTaskBuilder and allow to sort tasks using a new attribut execution_mode in ChainedTask, there's an issue about it as each task stored in a chained task does not receive a schedule date but it could be a good idea to allow sorting tasks 🤔

Plus, we can create a new PriorityPolicy which sort tasks depending on their priorities, WDYT?

[jvancoillie]

actually if we want to use orchestration, the getTasks() should return a TaskList, right?

I'm going to propose a pull request on a simple sort at the build of the chainedTask, is that ok?

[Guikingone]

The getTasks() should return a TaskList, right?

It could be a great step forward on the ChainedTask implementation, be careful as this could impact the TaskNormalizer but I think this proposal is a great one.

I'm going to propose a pull request on a simple sort at the build of the chainedTask, is that ok?

Feel free to 🙂

[jvancoillie]

I'm starting to think that maybe, we can inject the SchedulePolicyOrchestrator in the ChainedTaskBuilder and allow to sort tasks using a new attribut execution_mode in ChainedTask, there's an issue about it as each task stored in a chained task does not receive a schedule date but it could be a good idea to allow sorting tasks 🤔

Plus, we can create a new PriorityPolicy which sort tasks depending on their priorities, WDYT?

I replied before I got your feedback; I find your proposal more interesting

[jvancoillie]

I have proposed a pull request, can I integrate the PriorityPolicy on it or should I create a new one?

With this policy we could set it to a chained type by default.

I'm wondering about the validation of the configuration, currently there is no validation on the typeand name options of a task even though it is required, couldn't we improve this definition and validation ?

[Guikingone]

if you think that the policy is related, integrate it in the PR.

Feel free to improve things related to the PR 🙂

[Guikingone]

Hi @jvancoillie 👋🏻

Are we good with this discussion? 🤔

[jvancoillie]

yes i think it's ok, it was solved in the PR #89

[Guikingone]

Nice, thanks, I lock the conversation 🙂