Skip to content

New 2 handly functions#1229

Open
Arifursdev wants to merge 2 commits into
woocommerce:trunkfrom
Arifursdev:new-2-handly-functions
Open

New 2 handly functions#1229
Arifursdev wants to merge 2 commits into
woocommerce:trunkfrom
Arifursdev:new-2-handly-functions

Conversation

@Arifursdev

Copy link
Copy Markdown

This pull request introduces two new functions to the functions.php file to enhance the functionality of the action scheduler. These functions allow for running all queued actions or the next queued action for a given hook, arguments, and group.

New functionality for action scheduler:

  • Added as_run_queued_actions function to run all queued actions for a given hook, arguments, and group. This function initializes the action scheduler, queries the actions, and processes each action while handling exceptions.
  • Added as_run_next_queued_action function to run the next queued action for a given hook, arguments, and group. Similar to as_run_queued_actions, this function initializes the action scheduler, queries the next action, and processes it while handling exceptions.

@barryhughes barryhughes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Arifursdev sorry for the long delay. Can you confirm if you are still interested in driving this forward? If you are, please review the comments I left.

  • My biggest concern with this approach is the potential for a race condition leading to the same action being processed twice (because we skip the claim process/there is no awareness of existing claims).
  • New tests would be merited (see tests/phpunit/procedural_api/).
  • We'd also want to update the API ref at actionscheduler.org (see docs/api.md).
  • I definitely see some utility in these functions, but it would be great to hear about your specific use case, to help us make the right calls before committing this to the public API surface.

If you are no longer interested, no worries—we'll probably close the PR in due course.

Comment thread functions.php
Comment on lines +509 to +521
$params = array(
'hook' => $hook,
'status' => ActionScheduler_Store::STATUS_PENDING,
'orderby' => 'date',
'order' => 'ASC',
'group' => $group,
);

if (is_array($args)) {
$params['args'] = $args;
}

$actions = ActionScheduler::store()->query_actions($params);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A concern here is that we may pick up claimed actions (once an action is claimed, its status initially remains pending) that are on the verge of being processed by a conventional queue runner, leading to them being processed twice.

Comment thread functions.php
Comment on lines +553 to +565
$params = array(
'hook' => $hook,
'status' => ActionScheduler_Store::STATUS_PENDING,
'orderby' => 'date',
'order' => 'ASC',
'group' => $group,
);

if (is_array($args)) {
$params['args'] = $args;
}

$action_id = ActionScheduler::store()->query_action($params);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as before (possible race condition here).

Comment thread functions.php
* @param array $args Optional. An array of arguments to filter the action. Default is an empty array.
* @param string $group Optional. The group the action is assigned to. Default is an empty string.
*/
function as_run_next_queued_action($hook, $args = array(), $group = '') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For both functions, what are your thoughts on a return type? That might be a boolean for this function (because it is only interested in running a single action) or possibly an int for the other function (a count of how many actions were processed)?

Comment thread functions.php
* @param array $args Optional. An array of arguments to filter the actions. Default is an empty array.
* @param string $group Optional. The group the actions are assigned to. Default is an empty string.
*/
function as_run_queued_actions($hook, $args = array(), $group = '') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth running the new functions through the linter (composer run phpcs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants