Something which ought to be improved in my implementation is specifying 'enabled' cron tasks. I took the simplest approach that works: a config setting consisting of enabled task modules (to be changed to classes, probably) separated by commas:
standard,antispam_mod,thanks_mod
This approach however is not user-friendly:
1. There is no list of all possible modules that can be entered
1a. If a module is removed temporarily and is not added back, and later someone views this setting, it is virtually impossible to realize that a module is missing
2. It is easy to make a typo and add/remove a letter somewhere in this string, breaking functionality
Better UI is clearly desirable. A solution would need to account for modules installed by modifications.
[RFC|Merged] Modular cron
Re: System cron and cron tasks in mods
While I see some benefits of this, would you mind writing them down? Doing so will probably formalize some of the requirements and/or 'nice to have' items that the implementation can be judged against.eviL3 wrote:It would be nice if the tasks were more extensible/generic, with one file per task, either implementing or extending an interface or base class.
I only kept it because it was in original code. There is no other reason to keep it as a constant. In my implementation CRON_ID should actually be a plan instance variable.Perhaps CRON_ID can be defined outside of the cron_lock class or replaced with a static property (which isn't great either, but at least more namespaced).
Are we targeting 3.x line and php4 compatibility?
Re: System cron and cron tasks in mods
There are two main reasons for putting each task in it's own class. The code is less messy to begin with and follows a specific pattern. It allows you to add functionality simply by placing a new class in a specific folder, without editing any files.nn- wrote:While I see some benefits of this, would you mind writing them down? Doing so will probably formalize some of the requirements and/or 'nice to have' items that the implementation can be judged against.
There are additional benefits. The whole system is more extensible, because you can extend existing tasks, overwriting parts of them. You can apply a wide range of OO-patterns if needed. By defining an interface you decouple your code from any specific implementation, allowing any class to become a task.
You're right. I assumed it would be accessed somewhere from within phpBB3 since it's a constant, but this is not the case.nn- wrote:I only kept it because it was in original code. There is no other reason to keep it as a constant. In my implementation CRON_ID should actually be a plan instance variable.
I would say we are targeting 3.1 or 3.2, which will both be PHP 5.2. What a relief!nn- wrote:Are we targeting 3.x line and php4 compatibility?
Re: System cron and cron tasks in mods
I would say this should really target 3.1 and PHP 5.2+, should this be merged with the other cron topic?
Re: System cron and cron tasks in mods
Fine with me. nn-, your call.
Re: System cron and cron tasks in mods
I'm ok with merging.
Re: System cron and cron tasks in mods
I started redoing tasks as classes and I just realized that I changed all underscored function names to camel case. Should I change them back to underscores or do we eventually plan on moving toward camel case?
Re: System cron and cron tasks in mods
I doubt the naming convention will change with 3.x, so I'd go with underscores.
Re: System cron and cron tasks in mods
Proposed cron task interface: http://gist.github.com/366810#file_cron_task.php
Obviously method names would be changed to underscores.
Parametrized cron task is for prune single forum task, which is half cron task and half delayed job in that it is invoked explicitly in one location.
Implementation looks to be much cleaner now, I will commit it once I have renamed everything.
We need a naming convention for cron tasks. Do we organize them in modules (subdirectories)? How would they be discovered? How would users name them when deciding which tasks to enable?
Current structure is thus:
Obviously method names would be changed to underscores.
Parametrized cron task is for prune single forum task, which is half cron task and half delayed job in that it is invoked explicitly in one location.
Implementation looks to be much cleaner now, I will commit it once I have renamed everything.
We need a naming convention for cron tasks. Do we organize them in modules (subdirectories)? How would they be discovered? How would users name them when deciding which tasks to enable?
Current structure is thus:
Code: Select all
includes/cron_tasks/standard/prune_all_forums.php
includes/cron_tasks/standard/prune_forum.php
includes/cron_tasks/standard/queue.php
includes/cron_tasks/standard/tidy_cache.php
includes/cron_tasks/standard/tidy_database.php
includes/cron_tasks/standard/tidy_search.php
includes/cron_tasks/standard/tidy_sessions.php
includes/cron_tasks/standard/tidy_warnings.php
Re: System cron and cron tasks in mods
For the filenames, perhaps:
The class name being "cron_task_prune_all_forums". Or "cron_prune_all_forums". Or "phpbb_cron_prune_all_forums".
There is not much consistency within phpBB, looking at dbal, captcha, cache, auth plugins/modules. In the case of separating the "default" tasks from custom ones, I'd suggest using the term "core".
Interface looks good. Could you give a slightly more verbose description of how getUrlQueryString() works? In what cases would isShutdownFunctionSafe() return false? We should also take a look at the proposed dependency injection, I'd be glad to provide some more description of how that could work.
Code: Select all
includes/cron/tasks/prune_all_forums.php
There is not much consistency within phpBB, looking at dbal, captcha, cache, auth plugins/modules. In the case of separating the "default" tasks from custom ones, I'd suggest using the term "core".
Interface looks good. Could you give a slightly more verbose description of how getUrlQueryString() works? In what cases would isShutdownFunctionSafe() return false? We should also take a look at the proposed dependency injection, I'd be glad to provide some more description of how that could work.