diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php @@ -6,7 +6,7 @@ protected function didConstruct() { $this ->setName('cancel') - ->setExamples('**cancel** --id __id__') + ->setExamples('**cancel** __selectors__') ->setSynopsis( pht( 'Cancel selected tasks. The work these tasks represent will never '. @@ -15,14 +15,21 @@ } public function execute(PhutilArgumentParser $args) { - $console = PhutilConsole::getConsole(); $tasks = $this->loadTasks($args); + if (!$tasks) { + $this->logWarn( + pht('NO TASKS'), + pht('No tasks selected to cancel.')); + + return 0; + } + + $cancel_count = 0; foreach ($tasks as $task) { $can_cancel = !$task->isArchived(); if (!$can_cancel) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('ARCHIVED'), pht( '%s is already archived, and can not be cancelled.', @@ -32,20 +39,25 @@ // Forcibly break the lease if one exists, so we can archive the // task. - $task->setLeaseOwner(null); - $task->setLeaseExpires(PhabricatorTime::getNow()); - $task->archiveTask( - PhabricatorWorkerArchiveTask::RESULT_CANCELLED, - 0); - - $console->writeOut( - "** %s ** %s\n", + $task + ->setLeaseOwner(null) + ->setLeaseExpires(PhabricatorTime::getNow()); + + $task->archiveTask(PhabricatorWorkerArchiveTask::RESULT_CANCELLED, 0); + + $this->logInfo( pht('CANCELLED'), pht( '%s was cancelled.', $this->describeTask($task))); + + $cancel_count++; } + $this->logOkay( + pht('DONE'), + pht('Cancelled %s task(s).', new PhutilNumber($cancel_count))); + return 0; } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php @@ -6,7 +6,7 @@ protected function didConstruct() { $this ->setName('execute') - ->setExamples('**execute** --id __id__') + ->setExamples('**execute** __selectors__') ->setSynopsis( pht( 'Execute a task explicitly. This command ignores leases, is '. @@ -27,18 +27,24 @@ } public function execute(PhutilArgumentParser $args) { - $console = PhutilConsole::getConsole(); - $tasks = $this->loadTasks($args); - $is_retry = $args->getArg('retry'); $is_repeat = $args->getArg('repeat'); + $tasks = $this->loadTasks($args); + if (!$tasks) { + $this->logWarn( + pht('NO TASKS'), + pht('No tasks selected to execute.')); + + return 0; + } + + $execute_count = 0; foreach ($tasks as $task) { $can_execute = !$task->isArchived(); if (!$can_execute) { if (!$is_retry) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('ARCHIVED'), pht( '%s is already archived, and will not be executed. '. @@ -50,8 +56,7 @@ $result_success = PhabricatorWorkerArchiveTask::RESULT_SUCCESS; if ($task->getResult() == $result_success) { if (!$is_repeat) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('SUCCEEDED'), pht( '%s has already succeeded, and will not be retried. '. @@ -61,9 +66,8 @@ } } - echo tsprintf( - "** %s ** %s\n", - pht('ARCHIVED'), + $this->logInfo( + pht('UNARCHIVING'), pht( 'Unarchiving %s.', $this->describeTask($task))); @@ -74,30 +78,36 @@ // NOTE: This ignores leases, maybe it should respect them without // a parameter like --force? - $task->setLeaseOwner(null); - $task->setLeaseExpires(PhabricatorTime::getNow()); - $task->save(); + $task + ->setLeaseOwner(null) + ->setLeaseExpires(PhabricatorTime::getNow()) + ->save(); $task_data = id(new PhabricatorWorkerTaskData())->loadOneWhere( 'id = %d', $task->getDataID()); $task->setData($task_data->getData()); - echo tsprintf( - "%s\n", + $this->logInfo( + pht('EXECUTE'), pht( - 'Executing task %d (%s)...', - $task->getID(), - $task->getTaskClass())); + 'Executing %s...', + $this->describeTask($task))); $task = $task->executeTask(); - $ex = $task->getExecutionException(); + $ex = $task->getExecutionException(); if ($ex) { throw $ex; } + + $execute_count++; } + $this->logOkay( + pht('DONE'), + pht('Executed %s task(s).', new PhutilNumber($execute_count))); + return 0; } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementFreeWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementFreeWorkflow.php --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementFreeWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementFreeWorkflow.php @@ -6,7 +6,7 @@ protected function didConstruct() { $this ->setName('free') - ->setExamples('**free** --id __id__') + ->setExamples('**free** __selectors__') ->setSynopsis( pht( 'Free leases on selected tasks. If the daemon holding the lease is '. @@ -16,13 +16,20 @@ } public function execute(PhutilArgumentParser $args) { - $console = PhutilConsole::getConsole(); $tasks = $this->loadTasks($args); + if (!$tasks) { + $this->logWarn( + pht('NO TASKS'), + pht('No tasks selected to free leases on.')); + + return 0; + } + + $free_count = 0; foreach ($tasks as $task) { if ($task->isArchived()) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('ARCHIVED'), pht( '%s is archived; archived tasks do not have leases.', @@ -31,8 +38,7 @@ } if ($task->getLeaseOwner() === null) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('FREE'), pht( '%s has no active lease.', @@ -40,18 +46,24 @@ continue; } - $task->setLeaseOwner(null); - $task->setLeaseExpires(PhabricatorTime::getNow()); - $task->save(); + $task + ->setLeaseOwner(null) + ->setLeaseExpires(PhabricatorTime::getNow()) + ->save(); - $console->writeOut( - "** %s ** %s\n", + $this->logInfo( pht('LEASE FREED'), pht( '%s was freed from its lease.', $this->describeTask($task))); + + $free_count++; } + $this->logOkay( + pht('DONE'), + pht('Freed %s task lease(s).', new PhutilNumber($free_count))); + return 0; } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php @@ -6,7 +6,7 @@ protected function didConstruct() { $this ->setName('retry') - ->setExamples('**retry** --id __id__') + ->setExamples('**retry** __selectors__') ->setSynopsis( pht( 'Retry selected tasks which previously failed permanently or '. @@ -24,14 +24,21 @@ } public function execute(PhutilArgumentParser $args) { - $console = PhutilConsole::getConsole(); + $is_repeat = $args->getArg('repeat'); + $tasks = $this->loadTasks($args); + if (!$tasks) { + $this->logWarn( + pht('NO TASKS'), + pht('No tasks selected to retry.')); - $is_repeat = $args->getArg('repeat'); + return 0; + } + + $retry_count = 0; foreach ($tasks as $task) { if (!$task->isArchived()) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('ACTIVE'), pht( '%s is already in the active task queue.', @@ -42,8 +49,7 @@ $result_success = PhabricatorWorkerArchiveTask::RESULT_SUCCESS; if ($task->getResult() == $result_success) { if (!$is_repeat) { - $console->writeOut( - "** %s ** %s\n", + $this->logWarn( pht('SUCCEEDED'), pht( '%s has already succeeded, and will not be repeated. '. @@ -55,14 +61,19 @@ $task->unarchiveTask(); - $console->writeOut( - "** %s ** %s\n", + $this->logInfo( pht('QUEUED'), pht( '%s was queued for retry.', $this->describeTask($task))); + + $retry_count++; } + $this->logOkay( + pht('DONE'), + pht('Queued %s task(s) for retry.', new PhutilNumber($retry_count))); + return 0; } diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php @@ -14,16 +14,54 @@ array( 'name' => 'class', 'param' => 'name', - 'help' => pht('Select all tasks of a given class.'), + 'help' => pht('Select tasks of a given class.'), ), array( 'name' => 'min-failure-count', 'param' => 'int', - 'help' => pht('Limit to tasks with at least this many failures.'), + 'help' => pht('Select tasks with a minimum failure count.'), + ), + array( + 'name' => 'max-failure-count', + 'param' => 'int', + 'help' => pht('Select tasks with a maximum failure count.'), ), array( 'name' => 'active', - 'help' => pht('Select all active tasks.'), + 'help' => pht('Select active tasks.'), + ), + array( + 'name' => 'archived', + 'help' => pht('Select archived tasks.'), + ), + array( + 'name' => 'container', + 'param' => 'name', + 'help' => pht( + 'Select tasks with the given container or containers.'), + 'repeat' => true, + ), + array( + 'name' => 'object', + 'param' => 'name', + 'repeat' => true, + 'help' => pht( + 'Select tasks affecting the given object or objects.'), + ), + array( + 'name' => 'min-priority', + 'param' => 'int', + 'help' => pht('Select tasks with a minimum priority.'), + ), + array( + 'name' => 'max-priority', + 'param' => 'int', + 'help' => pht('Select tasks with a maximum priority.'), + ), + array( + 'name' => 'limit', + 'param' => 'int', + 'help' => pht('Limit selection to a maximum number of tasks.'), ), ); } @@ -31,14 +69,92 @@ protected function loadTasks(PhutilArgumentParser $args) { $ids = $args->getArg('id'); $class = $args->getArg('class'); - $min_failures = $args->getArg('min-failure-count'); $active = $args->getArg('active'); + $archived = $args->getArg('archived'); + + $container_names = $args->getArg('container'); + $object_names = $args->getArg('object'); + + $min_failures = $args->getArg('min-failure-count'); + $max_failures = $args->getArg('max-failure-count'); - if (!$ids && !$class && !$min_failures && !$active) { + $min_priority = $args->getArg('min-priority'); + $max_priority = $args->getArg('max-priority'); + + $limit = $args->getArg('limit'); + + $any_constraints = false; + if ($ids) { + $any_constraints = true; + } + + if ($class) { + $any_constraints = true; + } + + if ($active || $archived) { + $any_constraints = true; + if ($active && $archived) { + throw new PhutilArgumentUsageException( + pht( + 'You can not specify both "--active" and "--archived" tasks: '. + 'no tasks can match both constraints.')); + } + } + + if ($container_names) { + $any_constraints = true; + $container_phids = $this->loadObjectPHIDsFromArguments($container_names); + } else { + $container_phids = array(); + } + + if ($object_names) { + $any_constraints = true; + $object_phids = $this->loadObjectPHIDsFromArguments($object_names); + } else { + $object_phids = array(); + } + + if (($min_failures !== null) || ($max_failures !== null)) { + $any_constraints = true; + if (($min_failures !== null) && ($max_failures !== null)) { + if ($min_failures > $max_failures) { + throw new PhutilArgumentUsageException( + pht( + 'Specified "--min-failures" must not be larger than '. + 'specified "--max-failures".')); + } + } + } + + if (($min_priority !== null) || ($max_priority !== null)) { + $any_constraints = true; + if (($min_priority !== null) && ($max_priority !== null)) { + if ($min_priority > $max_priority) { + throw new PhutilArgumentUsageException( + pht( + 'Specified "--min-priority" may not be larger than '. + 'specified "--max-priority".')); + } + } + } + + if (!$any_constraints) { throw new PhutilArgumentUsageException( pht( - 'Use "--id", "--class", "--active", and/or "--min-failure-count" '. - 'to select tasks.')); + 'Use constraint flags (like "--id" or "--class") to select which '. + 'tasks to affect. Use "--help" for a list of supported constraint '. + 'flags.')); + } + + if ($limit !== null) { + $limit = (int)$limit; + if ($limit <= 0) { + throw new PhutilArgumentUsageException( + pht( + 'Specified "--limit" must be a positive integer.')); + } } $active_query = new PhabricatorWorkerActiveTaskQuery(); @@ -56,13 +172,35 @@ } if ($min_failures) { - $active_query = $active_query->withFailureCountBetween( - $min_failures, null); - $archive_query = $archive_query->withFailureCountBetween( - $min_failures, null); + $active_query->withFailureCountBetween($min_failures, $max_failures); + $archive_query->withFailureCountBetween($min_failures, $max_failures); + } + + if ($container_phids) { + $active_query->withContainerPHIDs($container_phids); + $archive_query->withContainerPHIDs($container_phids); + } + + if ($object_phids) { + $active_query->withObjectPHIDs($object_phids); + $archive_query->withObjectPHIDs($object_phids); + } + + if ($min_priority || $max_priority) { + $active_query->withPriorityBetween($min_priority, $max_priority); + $archive_query->withPriorityBetween($min_priority, $max_priority); + } + + if ($limit) { + $active_query->setLimit($limit); + $archive_query->setLimit($limit); } - $active_tasks = $active_query->execute(); + if ($archived) { + $active_tasks = array(); + } else { + $active_tasks = $active_query->execute(); + } if ($active) { $archive_tasks = array(); @@ -74,32 +212,22 @@ mpull($active_tasks, null, 'getID') + mpull($archive_tasks, null, 'getID'); + if ($limit) { + $tasks = array_slice($tasks, 0, $limit, $preserve_keys = true); + } + + if ($ids) { foreach ($ids as $id) { if (empty($tasks[$id])) { throw new PhutilArgumentUsageException( - pht('No task exists with id "%s"!', $id)); + pht('No task with ID "%s" matches the constraints!', $id)); } } } - if ($class && $min_failures) { - if (!$tasks) { - throw new PhutilArgumentUsageException( - pht('No task exists with class "%s" and at least %d failures!', - $class, - $min_failures)); - } - } else if ($class) { - if (!$tasks) { - throw new PhutilArgumentUsageException( - pht('No task exists with class "%s"!', $class)); - } - } else if ($min_failures) { - if (!$tasks) { - throw new PhutilArgumentUsageException( - pht('No tasks exist with at least %d failures!', $min_failures)); - } - } + + // We check that IDs are valid, but for all other constraints it is + // acceptable to select no tasks to act upon. // When we lock tasks properly, this gets populated as a side effect. Just // fake it when doing manual CLI stuff. This makes sure CLI yields have @@ -110,6 +238,20 @@ } } + // If the user specified one or more "--id" flags, process the tasks in + // the given order. Otherwise, process them in FIFO order so the sequence + // is somewhat consistent with natural execution order. + + // NOTE: When "--limit" is used, we end up selecting the newest tasks + // first. At time of writing, there's no way to order the queries + // correctly, so just accept it as reasonable behavior. + + if ($ids) { + $tasks = array_select_keys($tasks, $ids); + } else { + $tasks = msort($tasks, 'getID'); + } + return $tasks; } @@ -117,4 +259,52 @@ return pht('Task %d (%s)', $task->getID(), $task->getTaskClass()); } + private function loadObjectPHIDsFromArguments(array $names) { + $viewer = $this->getViewer(); + + $seen_names = array(); + foreach ($names as $name) { + if (isset($seen_names[$name])) { + throw new PhutilArgumentUsageException( + pht( + 'Object "%s" is specified more than once. Specify only unique '. + 'objects.', + $name)); + } + $seen_names[$name] = true; + } + + $object_query = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withNames($names); + + $object_query->execute(); + + $name_map = $object_query->getNamedResults(); + $phid_map = array(); + foreach ($names as $name) { + if (!isset($name_map[$name])) { + throw new PhutilArgumentUsageException( + pht( + 'No object with name "%s" could be loaded.', + $name)); + } + + $phid = $name_map[$name]->getPHID(); + + if (isset($phid_map[$phid])) { + throw new PhutilArgumentUsageException( + pht( + 'Names "%s" and "%s" identify the same object. Specify only '. + 'unique objects.', + $name, + $phid_map[$phid])); + } + + $phid_map[$phid] = $name; + } + + return array_keys($phid_map); + } + } diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerTaskQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerTaskQuery.php --- a/src/infrastructure/daemon/workers/query/PhabricatorWorkerTaskQuery.php +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerTaskQuery.php @@ -7,10 +7,13 @@ private $dateModifiedSince; private $dateCreatedBefore; private $objectPHIDs; + private $containerPHIDs; private $classNames; private $limit; private $minFailureCount; private $maxFailureCount; + private $minPriority; + private $maxPriority; public function withIDs(array $ids) { $this->ids = $ids; @@ -32,6 +35,11 @@ return $this; } + public function withContainerPHIDs(array $phids) { + $this->containerPHIDs = $phids; + return $this; + } + public function withClassNames(array $names) { $this->classNames = $names; return $this; @@ -43,6 +51,12 @@ return $this; } + public function withPriorityBetween($min, $max) { + $this->minPriority = $min; + $this->maxPriority = $max; + return $this; + } + public function setLimit($limit) { $this->limit = $limit; return $this; @@ -65,6 +79,13 @@ $this->objectPHIDs); } + if ($this->containerPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'containerPHID IN (%Ls)', + $this->containerPHIDs); + } + if ($this->dateModifiedSince !== null) { $where[] = qsprintf( $conn, @@ -100,6 +121,20 @@ $this->maxFailureCount); } + if ($this->minPriority !== null) { + $where[] = qsprintf( + $conn, + 'priority >= %d', + $this->minPriority); + } + + if ($this->maxPriority !== null) { + $where[] = qsprintf( + $conn, + 'priority <= %d', + $this->maxPriority); + } + return $this->formatWhereClause($conn, $where); }