TYPO3 Core

Francois's response....

The task is saved prior to actual execution because that triggers the calculation of the next execution date. Looking at this in retrospect doesn't make it look very clever. We would probably need 2 different methods: one that just saves the task and one that calculates next execution time and saves. That's a bit too late to have such a change in 4.4, but you may want to open a bug tracker issue so that we don't forget.

I'm unsure if it's a good idea to store such values in the task object in the first place, the registry might be a better choice for such things.

Imagine you'd like to show / use / manipulate those values at some other point in TYPO3: You'd have to de-serialize the task again to get the current state. If you store it in the registry instead, it is available in a transparent way wherever you want.

The only reason to store the serialized task instance in db is (afaik) to realize multiple instances of one task class with different settings (execution time, additional fields, ...). Task code which depends on other logic (external magic, return value of a previous run, ...) do not really fit and belong to this and should be handled separately.

I've changed by mind in contrast to my previous post.

I think it is a good idea to store some values in a task instance itself, and to update and persist them if needed. #16360 is a perfect use case: It stores and updates it's current pointer to a database row where the next task run should start from. This is done by just calling $this->save() at the end of execute(), which serializes and persists the task instance again.

@Jeff / @Francois Suter: Do you think this is sufficient? We could probably easily add a call to save() after execution of a task within the scheduler, but this will change behaviour and might introduce hard to debug issues to existing (third party) tasks. What do you think?

Patch set 1 of change I262584cdb4a896ea3b86be7c40fe184f2c55b724 has been pushed to the review server.
It is available at http://review.typo3.org/2479

Christian Kuhn wrote:

I'm unsure if it's a good idea to store such values in the task object in the first place, the registry might be a better choice for such things.

Imagine you'd like to show / use / manipulate those values at some other point in TYPO3: You'd have to de-serialize the task again to get the current state. If you store it in the registry instead, it is available in a transparent way wherever you want.

I agree with Christian, just use the registry, it's the perfect place for such things (last time data was fetched and so on...). We also do this with some EXT:solr scheduler tasks...

Okay. Finally you are right.
I`ve closed this patch and will wait for the updated ducomentation.

This might save others some time:
If you save the task twice in your code you need to restore the scheduler reference after saving:
$this->save();
$this->setScheduler();
as this gets unset while saving, but is actually used when saving again and will lead to a Fatal Error if its not present.

Apart from that you should unset large properties of your Task class before saving - in my case I needed to unset $this->objectManager and $this->persistenceManager which I used. Without unsetting them the task object gets destroyed - because its too large for DB serialization I guess.