In my application I never want the scheduler to run on cron so I am overriding the cron() method. Now that this class is separated out, it lets you select the class name you want to use but it has no way to let you include a file before creating the class like FeedsScheduler did - you must declare the class on every pageload.

Attached is a very simple patch that allows you to avoid this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Using http://drupal.org/project/autoload is probably a better bet...

makara’s picture

I think we can just use CTools, as in the patch. What do you think?

joachim’s picture

Hmm converting the jobscheduler classes to being CTools plugins?

That seems like quite a bit change... declaring classes in the .info files and letting autoloader load them is much simpler, plus it doesn't need forward porting as that's automatic in D7.

(Also, IIRC you need extra plugin code at the top of each class file.)

makara’s picture

Yes it is simpler to use autoload. But then we require a new module, which then adds more code, DB tables and PHP requirements. I'm not sure about it but I can imagine people would complain about it.

IIRC you need extra plugin code at the top of each class file

No, you need to provide a definition in a hook for each class, like the Views module does.

joachim’s picture

Autoload adds a dependency, but it's a module with zero configuration: you enable it and forget about it. I don't think that's a terrible imposition on users. Furthermore, quite a few D6 modules require it, so there's a chance users will have it already.

From a maintenance point of view, auytoload is a much lighter option. The D6 autoload is a backport of something built into D7 core, and that means that we don't carry a maintenance burden forward. We add a dependency to D6 and we add a bit of extra stuff to our info files. For D7, we drop the dependency, and the extra stuff in the info files we need for D7 anyway.

CTools plugins definitely do need some code at the top of the inc file that holds the class: looking in the ctools_plugin_example folder I see this:

/**
 * Plugins are described by creating a $plugin array which will be used
 * by the system that includes this file.
 */

There's also hooks that we need to implement to declare our plugin type and where they live and so on. Overall that's more core, and from a maintenance point of view it's code we need to carry forward to D7.

However, it could be there are advantages to using CTools. I'm not sure what we're trying to achieve here:

- do we just want a system for automatically loading our classes? If so, it seems to me that autoload is the right choice.
- do we want a system of plugins, where users can choose them from a list in the UI? For this we need not just to be able to load classes, but to be able to discover them. If that's what we want, then CTools is the right choice. But I don't know if it makes sense for users to set up jobs for which they pick the scheduler class.

makara’s picture

FileSize
1.59 KB

I agree that we don't need a full plugin system.

A new patch.

With this patch, we don't require the Autoload module, but people will need the module to use a custom job scheduler class.

twistor’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.