TYPO3 Core

Looks good, but I can't test it, since I can't create the registry table.

IIRC you want to display this information in the Reports module. I was thinking that it would be useful to display it in the Scheduler's BE module too, just above the table in the list view. I think it is particularly important now that I reverted some of the uses of $GLOBALS['EXEC_TIME'], in particular when registering when a task was run. Before that, you could see exactly when an execution had started, because all tasks carried the same timestamp, but this gave a wrong impression, since tasks are run in a certain order and some may be executed quite a bit later than the start of the run if previous tasks take a lot of time. Depending on what the tasks do, it is critical to have the time at which they were really executed and not the time at which the scheduler started running.

So I think it's good to display it in the BE module.

OK, I could finally test this patch, now that I could create the registry table.

It works fines, but I took a closer look at the patch itself and I don't think the call to the registry is placed correctly. Or maybe I misunderstood why you want to store the last run date. With the change you did, the last run date will be upated in the registry for each task that is executed. So if several tasks are executed in a given run, the registry will not contain the time of the run, but the time at which the last task was executed during that run. I don't think this information has any relevance. What would be more interesting IMO is to have the date of the beginning of the run and maybe an additional registry entry for the end of the run. That way we would have a precise idea of when it ran (which makes it possible to check whether that matches the cronjob, for example) and when it finished (giving an admin an idea of how much time his scheduled tasks normally require, although that will of course vary depending on which tasks are actually executed).

Furthermore, with your patch the registry will be updated also during manual executions from the BE module and again I don't think this is relevant. What is important is the date of the last automatic run, to check if everything went fine. So I think usage of the registry really belongs to cli/scheduler_cli_dispatch.php.

Does that make sense?

It works fines, but I took a closer look at the patch itself and I don't think the call to the registry is placed correctly. Or maybe I misunderstood why you want to store the last run date. With the change you did, the last run date will be upated in the registry for each task that is executed. So if several tasks are executed in a given run, the registry will not contain the time of the run, but the time at which the last task was executed during that run. I don't think this information has any relevance.

Right, I actually wanted to have the time when the execution of the scheduler started instead of each task. That place looked like it would be that.

Furthermore, with your patch the registry will be updated also during manual executions from the BE module and again I don't think this is relevant.

In fact it is! This is intended. In case someone does not have a cron job he can run his tasks manually and we still now when the scheduler ran last. That's what I'm going to check in the reports module.

Francois Suter wrote:

Done in r1189

It would have been nice if you could have waited for my feedback, especially as the task was assigned to me. I have to do day to day work, too. If you need quick feedback I'm available on skype though. I was also quite unhappy with the way you handled the refactoring of task registration without letting me know beforehand.

The way it is now is wrong IMO. On my dev system f.e. I do not have a cron set up and I now always have that message above the tasks, which is nonsense as that scheduler will never run automatically. The same applies to others without cron as well of course.
We'll have to change that.

Yes, I could have waited for your feedback. Sorry. But I also have very little time this week and I tried to make progress whenever I could.

About the registry, I would say it makes sense to have both pieces of information:

- the start and end of last automatic run, as I implented, because it is a very useful information
- some information about the manual runs, which is important too. In this case I propose to store the same kind of information as for the automatic run. To achieve this, the same kind of calls need to be done as in the dispatch script, but inside the BE module's executeTasks() method. Apart from this, the last run date for each task is available in the DB anyway.

I see your point about having the warning all the time if you have no cron set up. I must say I did not even think that such a setup could make sense... Maybe this could be an option: a checkbox at the bottom of the list view, to toggle the display of that message. That message should probably go into the Reports module, but - as an admin - I think I would appreciate being able to see it in the Scheduler's BE module too.

How does that sound?

Francois Suter wrote:

As discussed per Skype, I implemented the following (in r1195):

- I added the type of run (manual or cron) to the information stored in the registry
- I moved the display of that information to the check screen (locallang labels were adapted/added as necessary)

Since this last run information is now written both after CLI and manual executions, I added a new method (recordLastRun()) to the tx_scheduler object, to encapsulate that logic.

Also, I don't think anyone will want to access the start time, end time or run type information separately. Consequently I put all the information into an array and stored that, so that the registry is called a single time rather than three times.

looks good, good catch!