Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
See patch for my suggested fix.
I tried to create a batch job but had a hard time getting it to work with the documentation provided. It ended up that the function I registered to run in the batch was not normally included, so I had to specify that file to be included.
I patched the documentation to show that you can define a file to be included in a batch run
Please use this patch or something similar. There are 0 errors about function not found. It just runs the batch as if everything ran without incident.
Thank you!
Mark
Comment | File | Size | Author |
---|---|---|---|
#15 | 528822-D6.patch | 4.66 KB | jhodgdon |
#8 | 528822.patch | 5.12 KB | jhodgdon |
#6 | 528822.patch | 4.68 KB | jhodgdon |
Batch.include.file_.diff | 410 bytes | markDrupal | |
Comments
Comment #1
JirkaRybka CreditAttribution: JirkaRybka commentedAnother unclear point is the $context['sandbox'], which is NOT persistent in the scope of the whole batch, i.e. it's cleared after every single operation in the batch. Only when the same operation is called multiple times (which is not default), it can benefit from sandbox - otherwise the batch persistent data needs to be stored (awkwardly) in results instead.
Comment #2
jhodgdonWhich function or api doc page are you patching? Your patch is difficult to review as it is...
Comment #3
markDrupal CreditAttribution: markDrupal commentedIt's the comments for the batch_set($batch_definition) function
Comment #4
jhodgdonIt's poorly formatted on api.drupal.org (the components should have - in front of each one, to format as a bullet list), but it looks like the API documentation for http://api.drupal.org/api/function/batch_set/6 does say that one component is:
'file': the path to the file containing the definitions of the
'operations' and 'finished' functions, for instance if they don't
reside in the original '.module' file. The path should be relative to
the base_path(), and thus should be built using drupal_get_path().
It looks like you are asking for the documentation on this page http://api.drupal.org/api/group/batch/6 to be updated with the optional 'file' parameter in its example?
I wonder how common it is that the functions in the batch_set() call would not be in the main module file (the example doesn't do anything with many of the optional parameters), and (assuming the formatting was fixed) whether the above documentation in batch_set() is enough?
Anyway, if this is to be patched, I think the formatting issue in batch_set() should also be fixed, and it should also be patched in Drupal 7 first and then back ported to Drupal 6.
Comment #5
markDrupal CreditAttribution: markDrupal commentedyeah the example (http://api.drupal.org/api/group/batch/6) is what I had problems with, after walking through the example, I got no feedback in the form of error messages from the website that my batch was not run, and given that it is a batch job it was difficult to debug and pin point the error back to the missing 'file' declaration
BTW, I thought Drupal 7 has some sort of auto loading of files feature so this may not be required any more after 6.
Comment #6
jhodgdonDrupal 7's function registry was removed -- they found it didn't help with performance.
Anyway, here's a patch for the doc in Drupal 7. Should fix the formatting issues on batch_set() and also adds a file component to the example in the overall batch group doc.
If accepted, should be ported to D6 as well.
Comment #8
jhodgdonHere's a patch reroll. Also needs port to Drupal 6 if accepted.
Comment #9
yched CreditAttribution: yched commentedPatch looks good except for the 3rd person in the one-line descriptions in PHPdocs.
Comment #10
jhodgdonThey are supposed to be 3rd person. See http://drupal.org/node/1354
Comment #11
yched CreditAttribution: yched commentedI stand corrected: http://drupal.org/node/487802#comment-1740560.
Patch is ready, then.
Side note: http://drupal.org/node/1354 also says
Implements hook_help().
while everything in core D7 still usesImplement hook_help().
, so it kind of questions the validity of the doc. Pointing to webchick's decision linked above could be clearer in-between ;-)Comment #12
webchickYes, I've been desperately wanting a patch to bulk-change those all to "Implements" since the documentation was updated. Sigh.
Anyway, committed to HEAD. :) Moving down to 6.x.
Comment #13
jhodgdonI hear you webchick. Will consider doing such a patch. Do we have an issue already?
Comment #14
jhodgdonThe D7 "fix all the Implement hook_foo()" issue:
#502190: Hook implementation headers out of compliance with standards
if anyone is interested.
Comment #15
jhodgdonHere's a patch for Drupal 6. It appears that the 'css' and 'url_options' components are new to Drupal 7; otherwise, the resulting doc is the same as the committed D7 patch, I think.
Comment #16
markDrupal CreditAttribution: markDrupal commentedAwesome! Thanks for getting this committed.
Comment #17
jhodgdonThe Drupal 7 patch has been committed. The Drupal 6 patch still needs someone other than me to mark it "Reviewed and Tested by the Community" before it can be committed.
Comment #18
yched CreditAttribution: yched commentedPatch is correct, except *D6* does not use 3rd person for function descriptors ;-).
Comment #19
jhodgdonD6 is inconsistent in this regard. There was no standard for this when D6 was released, and if you look at function headers for D6, you'll find a mix. So, when updating function headers, I tend to update them to our current coding standards, which are not marked anywhere as being version-dependent by the way! :)
Comment #20
yched CreditAttribution: yched commentedMakes sense. You win again ;-)
Comment #21
Gábor HojtsyCommitted to Drupal 6, thanks.