[RFC|Merged] Modular cron

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC] Modular cron

Post by Oleg » Sun Jul 11, 2010 12:56 pm

rxu, eviL3: there is no ultimatum here. I am merely explaining the situation (namely, lack of my activity here) for those who might be following this topic. The development team is free to do whatever they want, just as I am free to do whatever I want. If anyone wants me to continue working on the modular cron and/or other things I explained what has to happen.

bantu: thank you for your comments, I will make the requested changes after my other tickets are attended to (per #2 above).

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC] Modular cron

Post by Oleg » Sun Jul 11, 2010 12:59 pm

Testing via system cron:

1. Apply the patch.
2. Go to acp, I believe it's under server settings, change cron type from phpbb to system.
3. Invoke cron.php. To test in correct environment, do it from console. With that, the code should still work if invoked via curl or web browser.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Modular cron

Post by bantu » Mon Jul 12, 2010 12:35 am

bantu wrote:Although PHP lacks try/finally, wouldn't it be possible to unlock cron using register_shutdown_function() if available?
This seems to be what the current 3.0.x code is doing.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC] Modular cron

Post by Oleg » Mon Jul 12, 2010 5:06 pm

I wrote the code in question a while ago obviously but if I remember correctly the issue is this bit from php manual:
Multiple calls to register_shutdown_function() can be made, and each will be called in the same order as they were registered.
DB connection is closed in a shutdown function registered before lock release, so if lock is not released by the end of normal script execution it will not be released in the shutdown function either.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Thu Aug 19, 2010 3:00 am

As requested I cleaned up the history: removed merge commits, squashed fix-up commits, also changed one instance of intval() call to (int) cast.

http://github.com/p/phpbb3/compare/deve ... ystem-cron

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Thu Aug 19, 2010 8:34 pm

Reviewed it, looks good. I tested it a little and it appears to work.

Debug information on using system_cron when DEBUG_EXTRA is enabled. Telling which cron tasks were executed.

viewforum.php:

Code: Select all

if ($task && $task->is_ready()) {
Needs to be on a new line.

cron_manager:

Code: Select all

				if (!class_exists($class))
				{
					include($phpbb_root_path . "includes/cron/tasks/$mod/$filename.$phpEx");
				}
Should use the new autoloading. This means that tasks needs to be renamed to task. That should be all to get it working.

http://github.com/evil3/phpbb3/commit/4 ... 63d1c4dff9

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Thu Aug 19, 2010 8:36 pm

Now we need some documentation on the wiki...

http://wiki.phpbb.com/display/DEV/Modular+cron

User avatar
ToonArmy
Registered User
Posts: 335
Joined: Fri Mar 26, 2004 7:31 pm
Location: Bristol, UK
Contact:

Re: [RFC|Accepted] Modular cron

Post by ToonArmy » Fri Aug 20, 2010 2:59 pm

Really liking the look of this, few things I've noticed:
  • Some class methods don't have a visibility defined.
  • A number of classes are included manually
  • The cron_task interface file has another interface in the file
Chris SmithBlogXMOOhlohArea51WikiNo support via PM/IM
Image

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Fri Sep 10, 2010 8:50 am

eviL3 wrote:Should use the new autoloading. This means that tasks needs to be renamed to task. That should be all to get it working.
Bumping this.

includes/cron/cron_manager.php would become includes/cron/manager.php, the class name would become phpbb_cron_manager. And you could get rid of all of the includes.

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Sat Sep 11, 2010 2:47 am

I am addressing reported issues/requests as time permits.

Outstanding requests are currently as follows:

- Use new naming standards (naderman, bantu)
- prefix classes with phpbb_
- rename tasks to task
- cron task interface has another interface in file (toonarmy)
- Use autoloading (naderman, bantu, evil3, toonarmy) http://tracker.phpbb.com/browse/PHPBB3- ... tion_34289
- Tests for new and/or changed code (naderman)
- Add @return and @param documentation to every function (bantu)
- Add documentation to wiki (evil3)

Some of this is simply mechanical work, although there is a fair amount of it due to the size of the diff.

As far as renaming classes goes, if that is done the code that loads tasks may need to be adjusted since it loads both phpbb classes and non-phpbb classes.

Last time I tried running unit tests I did not succeed.

Already addressed:

- Updated install/schema_data.sql (bantu)
- Use integer casting instead of intval (bantu)
- Added debug information (evil3)
- Fixed braces (evil3)
- Added missing visibility specifiers (toonarmy)

Post Reply