From 620f6b8d1ede846388f0fd1ec04846edf7db0248 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Thu, 17 Aug 2017 16:48:48 +0200 Subject: [PATCH 1/4] added additional field to execution --- composer.json | 2 +- src/Resources/config/doctrine/TaskExecution.orm.xml | 3 ++- src/Resources/config/task_event_listener.xml | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 949e4c5..61525af 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ ], "require": { "php": "~5.5 || ~7.0", - "php-task/php-task": "dev-master", + "php-task/php-task": "dev-feature/retry-execution", "symfony/http-kernel": "^2.6 || ^3.0", "symfony/dependency-injection": "^2.6 || ^3.0", "symfony/expression-language": "^2.6 || ^3.0", diff --git a/src/Resources/config/doctrine/TaskExecution.orm.xml b/src/Resources/config/doctrine/TaskExecution.orm.xml index ac1aa8a..54ece00 100644 --- a/src/Resources/config/doctrine/TaskExecution.orm.xml +++ b/src/Resources/config/doctrine/TaskExecution.orm.xml @@ -22,7 +22,8 @@ - + + diff --git a/src/Resources/config/task_event_listener.xml b/src/Resources/config/task_event_listener.xml index 184eb25..7dad9b1 100644 --- a/src/Resources/config/task_event_listener.xml +++ b/src/Resources/config/task_event_listener.xml @@ -11,6 +11,7 @@ Task\Event\Events::TASK_FINISHED Task\Event\Events::TASK_PASSED Task\Event\Events::TASK_FAILED + Task\Event\Events::TASK_RETRIED From 2a5f895163141c6e4d7426e1ed1a6c269cc21b54 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Fri, 18 Aug 2017 13:51:04 +0200 Subject: [PATCH 2/4] added retry to separate-process executor --- src/Command/ExecuteCommand.php | 12 +- src/Command/RunCommand.php | 1 + src/Executor/ExecutionProcessFactory.php | 46 ++++ src/Executor/SeparateProcessExecutor.php | 112 ++++++++-- src/Resources/config/executor/separate.xml | 6 + .../Executor/SeparateProcessExecutorTest.php | 205 ++++++++++++++++++ 6 files changed, 364 insertions(+), 18 deletions(-) create mode 100644 src/Executor/ExecutionProcessFactory.php create mode 100644 tests/Unit/Executor/SeparateProcessExecutorTest.php diff --git a/src/Command/ExecuteCommand.php b/src/Command/ExecuteCommand.php index 554e104..e45fea3 100644 --- a/src/Command/ExecuteCommand.php +++ b/src/Command/ExecuteCommand.php @@ -16,6 +16,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Task\Executor\FailedException; use Task\Handler\TaskHandlerFactoryInterface; use Task\Storage\TaskExecutionRepositoryInterface; @@ -63,15 +64,20 @@ protected function configure() */ protected function execute(InputInterface $input, OutputInterface $output) { - $errOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + $errorOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; $execution = $this->executionRepository->findByUuid($input->getArgument('uuid')); $handler = $this->handlerFactory->create($execution->getHandlerClass()); try { $result = $handler->handle($execution->getWorkload()); - } catch (\Exception $e) { - $errOutput->writeln($e->__toString()); + } catch (\Exception $exception) { + if ($exception instanceof FailedException) { + $errorOutput->writeln(FailedException::class); + $exception = $exception->getPrevious(); + } + + $errorOutput->writeln($exception->__toString()); // Process exit-code: 0 = OK, >1 = FAIL return 1; diff --git a/src/Command/RunCommand.php b/src/Command/RunCommand.php index 6535b35..4c5fdf3 100644 --- a/src/Command/RunCommand.php +++ b/src/Command/RunCommand.php @@ -14,6 +14,7 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Task\Runner\ExitException; use Task\Runner\TaskRunnerInterface; use Task\Scheduler\TaskSchedulerInterface; diff --git a/src/Executor/ExecutionProcessFactory.php b/src/Executor/ExecutionProcessFactory.php new file mode 100644 index 0000000..0649381 --- /dev/null +++ b/src/Executor/ExecutionProcessFactory.php @@ -0,0 +1,46 @@ +consolePath = $consolePath; + $this->environment = $environment; + } + + /** + * Create process for given execution-uuid. + * + * @param string $uuid + * + * @return Process + */ + public function create($uuid) + { + return $process = ProcessBuilder::create( + [$this->consolePath, 'task:execute', $uuid, '-e ' . $this->environment] + )->getProcess(); + } +} diff --git a/src/Executor/SeparateProcessExecutor.php b/src/Executor/SeparateProcessExecutor.php index 610c38a..8e9b89d 100644 --- a/src/Executor/SeparateProcessExecutor.php +++ b/src/Executor/SeparateProcessExecutor.php @@ -11,9 +11,12 @@ namespace Task\TaskBundle\Executor; -use Symfony\Component\Process\ProcessBuilder; use Task\Execution\TaskExecutionInterface; use Task\Executor\ExecutorInterface; +use Task\Executor\FailedException; +use Task\Executor\RetryTaskHandlerInterface; +use Task\Handler\TaskHandlerFactoryInterface; +use Task\Storage\TaskExecutionRepositoryInterface; /** * Uses a separate process to start the executions via console-command. @@ -21,23 +24,33 @@ class SeparateProcessExecutor implements ExecutorInterface { /** - * @var string + * @var TaskHandlerFactoryInterface */ - private $consolePath; + private $handlerFactory; /** - * @var string + * @var TaskExecutionRepositoryInterface */ - private $environment; + private $executionRepository; /** - * @param string $consolePath - * @param string $environment + * @var ExecutionProcessFactory */ - public function __construct($consolePath, $environment) - { - $this->consolePath = $consolePath; - $this->environment = $environment; + private $processFactory; + + /** + * @param TaskHandlerFactoryInterface $handlerFactory + * @param TaskExecutionRepositoryInterface $executionRepository + * @param ExecutionProcessFactory $processFactory + */ + public function __construct( + TaskHandlerFactoryInterface $handlerFactory, + TaskExecutionRepositoryInterface $executionRepository, + ExecutionProcessFactory $processFactory + ) { + $this->handlerFactory = $handlerFactory; + $this->executionRepository = $executionRepository; + $this->processFactory = $processFactory; } /** @@ -45,16 +58,85 @@ public function __construct($consolePath, $environment) */ public function execute(TaskExecutionInterface $execution) { - $process = ProcessBuilder::create( - [$this->consolePath, 'task:execute', $execution->getUuid(), '-e ' . $this->environment] - )->getProcess(); + $attempts = $this->getMaximumAttempts($execution->getHandlerClass()); + $lastException = null; + + for ($attempt = 0; $attempt < $attempts; $attempt++) { + try { + return $this->handle($execution); + } catch (FailedException $exception) { + throw $exception; + } catch (SeparateProcessException $exception) { + if ($execution->getAttempts() < $attempts) { + $execution->incrementAttempts(); + $this->executionRepository->save($execution); + } + + $lastException = $exception; + } + } + + // maximum attempts to pass executions are reached + throw new FailedException($lastException); + } + + /** + * Returns maximum attempts for specified handler. + * + * @param string $handlerClass + * + * @return int + */ + private function getMaximumAttempts($handlerClass) + { + $handler = $this->handlerFactory->create($handlerClass); + if (!$handler instanceof RetryTaskHandlerInterface) { + return 1; + } + return $handler->getMaximumAttempts(); + } + + /** + * Handle execution by using console-command. + * + * @param TaskExecutionInterface $execution + * + * @return string + * + * @throws FailedException + * @throws SeparateProcessException + */ + private function handle(TaskExecutionInterface $execution) + { + $process = $this->processFactory->create($execution->getUuid()); $process->run(); if (!$process->isSuccessful()) { - throw new SeparateProcessException($process->getErrorOutput()); + throw $this->createException($process->getErrorOutput()); } return $process->getOutput(); } + + /** + * Create the correct exception. + * + * FailedException for failed executions. + * SeparateProcessExceptions for any exception during execution. + * + * @param string $errorOutput + * + * @return FailedException|SeparateProcessException + */ + private function createException($errorOutput) + { + if (strpos($errorOutput, FailedException::class) !== 0) { + return new SeparateProcessException($errorOutput); + } + + $errorOutput = trim(str_replace(FailedException::class, '', $errorOutput)); + + return new FailedException(new SeparateProcessException($errorOutput)); + } } diff --git a/src/Resources/config/executor/separate.xml b/src/Resources/config/executor/separate.xml index 217f23f..040a447 100644 --- a/src/Resources/config/executor/separate.xml +++ b/src/Resources/config/executor/separate.xml @@ -4,6 +4,12 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + + + + + + %task.executor.console_path% %kernel.environment%% diff --git a/tests/Unit/Executor/SeparateProcessExecutorTest.php b/tests/Unit/Executor/SeparateProcessExecutorTest.php new file mode 100644 index 0000000..8dbfaa5 --- /dev/null +++ b/tests/Unit/Executor/SeparateProcessExecutorTest.php @@ -0,0 +1,205 @@ +handlerFactory = $this->prophesize(TaskHandlerFactoryInterface::class); + $this->executionRepository = $this->prophesize(TaskExecutionRepositoryInterface::class); + $this->processFactory = $this->prophesize(ExecutionProcessFactory::class); + + $this->executor = new SeparateProcessExecutor( + $this->handlerFactory->reveal(), $this->executionRepository->reveal(), $this->processFactory->reveal() + ); + + $this->handler = $this->prophesize(TaskHandlerInterface::class); + $this->retryHandler = $this->prophesize(TaskHandlerInterface::class); + $this->retryHandler->willImplement(RetryTaskHandlerInterface::class); + + $this->handlerFactory->create('TaskHandler')->willReturn($this->handler->reveal()); + $this->handlerFactory->create('RetryTaskHandler')->willReturn($this->retryHandler->reveal()); + + $this->execution = $this->prophesize(TaskExecutionInterface::class); + $this->execution->getUuid()->willReturn('123-123-123'); + $this->execution->getAttempts()->willReturn(1); + + $this->process = $this->prophesize(Process::class); + $this->processFactory->create('123-123-123')->willReturn($this->process->reveal()); + } + + public function testExecute() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(true); + + $this->process->getOutput()->willReturn('TEST'); + + $result = $this->executor->execute($this->execution->reveal()); + $this->assertEquals('TEST', $result); + } + + public function testExecuteException() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn('TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteFailedException() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn(FailedException::class . PHP_EOL . 'TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteRetryFailedException() + { + $this->execution->getHandlerClass()->willReturn('RetryTaskHandler'); + $this->retryHandler->getMaximumAttempts()->willReturn(3); + + $this->execution->incrementAttempts()->shouldNotBeCalled(); + + $this->process->run()->shouldBeCalledTimes(1); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn(FailedException::class . PHP_EOL . 'TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteRetryException() + { + $this->execution->getHandlerClass()->willReturn('RetryTaskHandler'); + $this->retryHandler->getMaximumAttempts()->willReturn(3); + + $attempts = 1; + $this->execution->incrementAttempts()->will( + function () use (&$attempts) { + $attempts++; + + return $this; + } + ); + $this->execution->getAttempts()->will( + function () use (&$attempts) { + return $attempts; + } + ); + + $this->executionRepository->save($this->execution->reveal())->shouldBeCalled(2); + + $this->process->run()->shouldBeCalledTimes(3); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn('TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + $this->assertEquals(3, $attempts); + } + } +} From e289462cafe08990157e886a2edcb0a98615f11e Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Tue, 22 Aug 2017 08:28:53 +0200 Subject: [PATCH 3/4] added upgrade note --- UPGRADE.md | 10 ++++++++++ src/Command/RunCommand.php | 1 - src/Executor/SeparateProcessExecutor.php | 2 +- tests/Unit/Executor/SeparateProcessExecutorTest.php | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 25a2b82..2a79fbb 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,9 +1,19 @@ UPGRADE ======= +- [1.2.0](#1.2.0) - [1.1.0](#1.1.0) - [0.4.0](#0.4.0) +### 1.2.0 + +In the database table `ta_task_executions` a new field was introduced. Run following +command to update the table. + +```bash +bin/console doctrine:schema:update +``` + ### 1.1.0 In the database table `ta_tasks` a new field was introduced. Run following diff --git a/src/Command/RunCommand.php b/src/Command/RunCommand.php index 4c5fdf3..6535b35 100644 --- a/src/Command/RunCommand.php +++ b/src/Command/RunCommand.php @@ -14,7 +14,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use Task\Runner\ExitException; use Task\Runner\TaskRunnerInterface; use Task\Scheduler\TaskSchedulerInterface; diff --git a/src/Executor/SeparateProcessExecutor.php b/src/Executor/SeparateProcessExecutor.php index 8e9b89d..cd56c86 100644 --- a/src/Executor/SeparateProcessExecutor.php +++ b/src/Executor/SeparateProcessExecutor.php @@ -61,7 +61,7 @@ public function execute(TaskExecutionInterface $execution) $attempts = $this->getMaximumAttempts($execution->getHandlerClass()); $lastException = null; - for ($attempt = 0; $attempt < $attempts; $attempt++) { + for ($attempt = 0; $attempt < $attempts; ++$attempt) { try { return $this->handle($execution); } catch (FailedException $exception) { diff --git a/tests/Unit/Executor/SeparateProcessExecutorTest.php b/tests/Unit/Executor/SeparateProcessExecutorTest.php index 8dbfaa5..e7311c2 100644 --- a/tests/Unit/Executor/SeparateProcessExecutorTest.php +++ b/tests/Unit/Executor/SeparateProcessExecutorTest.php @@ -173,7 +173,7 @@ public function testExecuteRetryException() $attempts = 1; $this->execution->incrementAttempts()->will( function () use (&$attempts) { - $attempts++; + ++$attempts; return $this; } From eef7e9362be4dda950b28efbd20afb176041f73c Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Tue, 22 Aug 2017 14:52:07 +0200 Subject: [PATCH 4/4] updated dependencies --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 61525af..949e4c5 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ ], "require": { "php": "~5.5 || ~7.0", - "php-task/php-task": "dev-feature/retry-execution", + "php-task/php-task": "dev-master", "symfony/http-kernel": "^2.6 || ^3.0", "symfony/dependency-injection": "^2.6 || ^3.0", "symfony/expression-language": "^2.6 || ^3.0",