From c94b28555df46f0d12b1c3be1fb84abbba866e52 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 27 Jan 2022 09:18:01 +0000 Subject: [PATCH 1/8] Remove Try Catch's and use global handler --- app/executor.php | 225 ++++++++++++++++++++--------------------------- 1 file changed, 97 insertions(+), 128 deletions(-) diff --git a/app/executor.php b/app/executor.php index 59c5de8f0..1c7d49f84 100644 --- a/app/executor.php +++ b/app/executor.php @@ -915,32 +915,24 @@ function runBuildStage(string $buildId, string $projectID): Document App::post('/v1/execute') // Define Route ->desc('Execute a function') - ->param('trigger', '', new Text(1024)) - ->param('projectId', '', new Text(1024)) - ->param('executionId', '', new Text(1024), '', true) - ->param('functionId', '', new Text(1024)) - ->param('event', '', new Text(1024), '', true) - ->param('eventData', '', new Text(10240), '', true) - ->param('data', '', new Text(1024), '', true) - ->param('webhooks', [], new ArrayList(new JSON()), '', true) - ->param('userId', '', new Text(1024), '', true) - ->param('jwt', '', new Text(1024), '', true) + ->param('trigger', '', new Text(1024), 'What triggered this execution, can be http / schedule / event') + ->param('projectId', '', new Text(1024), 'The ProjectID this execution belongs to') + ->param('executionId', '', new Text(1024), 'An optional execution ID, If not specified a new execution document is created.', true) + ->param('functionId', '', new Text(1024), 'The FunctionID to execute') + ->param('event', '', new Text(1024), 'The event that triggered this execution', true) + ->param('eventData', '', new Text(0), 'Extra Data for the event', true) + ->param('data', '', new Text(1024), 'Data to be forwarded to the function, this is user specified.', true) + ->param('webhooks', [], new ArrayList(new JSON()), 'Any webhooks that need to be triggered after this execution', true) + ->param('userId', '', new Text(1024), 'The UserID of the user who triggered the execution if it was called from a client SDK', true) + ->param('jwt', '', new Text(1024), 'A JWT of the user who triggered the execution if it was called from a client SDK', true) ->inject('response') ->inject('dbForProject') ->action( function (string $trigger, string $projectId, string $executionId, string $functionId, string $event, string $eventData, string $data, array $webhooks, string $userId, string $jwt, Response $response, Database $dbForProject) { - try { - $data = execute($trigger, $projectId, $executionId, $functionId, $dbForProject, $event, $eventData, $data, $webhooks, $userId, $jwt); - $response->json($data); - } catch (Exception $e) { - logError($e, 'executeEndpoint'); + $data = execute($trigger, $projectId, $executionId, $functionId, $dbForProject, $event, $eventData, $data, $webhooks, $userId, $jwt); - $response - ->addHeader('Cache-Control', 'no-cache, no-store, must-revalidate') - ->addHeader('Expires', '0') - ->addHeader('Pragma', 'no-cache') - ->json(['error' => $e->getMessage()]); - } + $response->setStatusCode(Response::STATUS_CODE_OK); + $response->json($data); } ); @@ -951,10 +943,9 @@ App::post('/v1/cleanup/function') ->inject('dbForProject') ->action( function (string $functionId, Response $response, Database $dbForProject) use ($orchestrationPool) { + /** @var Orchestration $orchestration */ + $orchestration = $orchestrationPool->get(); try { - /** @var Orchestration $orchestration */ - $orchestration = $orchestrationPool->get(); - // Get function document $function = $dbForProject->getDocument('functions', $functionId); @@ -967,38 +958,35 @@ App::post('/v1/cleanup/function') // If amount is 0 then we simply return true if (count($results) === 0) { + $response->setStatusCode(Response::STATUS_CODE_OK); return $response->json(['success' => true]); } // Delete the containers of all deployments foreach ($results as $deployment) { - try { - // Remove any ongoing builds - if ($deployment->getAttribute('buildId')) { - $build = $dbForProject->getDocument('builds', $deployment->getAttribute('buildId')); + // Remove any ongoing builds + if ($deployment->getAttribute('buildId')) { + $build = $dbForProject->getDocument('builds', $deployment->getAttribute('buildId')); - if ($build->getAttribute('status') === 'building') { - // Remove the build - $orchestration->remove('build-stage-' . $deployment->getAttribute('buildId'), true); - Console::info('Removed build for deployment ' . $deployment['$id']); - } + if ($build->getAttribute('status') === 'building') { + // Remove the build + $orchestration->remove('build-stage-' . $deployment->getAttribute('buildId'), true); + Console::info('Removed build for deployment ' . $deployment['$id']); } - - $orchestration->remove('appwrite-function-' . $deployment['$id'], true); - Console::info('Removed container for deployment ' . $deployment['$id']); - } catch (Exception $e) { - // Do nothing, we don't care that much if it fails } + + $orchestration->remove('appwrite-function-' . $deployment['$id'], true); + Console::info('Removed container for deployment ' . $deployment['$id']); } + $response->setStatusCode(Response::STATUS_CODE_OK); return $response->json(['success' => true]); - } catch (Exception $e) { - logError($e, "cleanupFunction"); + } catch (Throwable $th) { + $orchestrationPool->put($orchestration); + throw $th; + } finally { $orchestrationPool->put($orchestration); - - return $response->json(['error' => $e->getMessage()]); } - $orchestrationPool->put($orchestration); } ); @@ -1007,10 +995,9 @@ App::post('/v1/cleanup/deployment') ->inject('response') ->inject('dbForProject') ->action(function (string $deploymentId, Response $response, Database $dbForProject) use ($orchestrationPool) { + /** @var Orchestration $orchestration */ + $orchestration = $orchestrationPool->get(); try { - /** @var Orchestration $orchestration */ - $orchestration = $orchestrationPool->get(); - // Get deployment document $deployment = $dbForProject->getDocument('deployments', $deploymentId); @@ -1019,29 +1006,23 @@ App::post('/v1/cleanup/deployment') throw new Exception('Deployment not found', 404); } - try { - // Remove any ongoing builds - if ($deployment->getAttribute('buildId')) { - $build = $dbForProject->getDocument('builds', $deployment->getAttribute('buildId')); + // Remove any ongoing builds + if ($deployment->getAttribute('buildId')) { + $build = $dbForProject->getDocument('builds', $deployment->getAttribute('buildId')); - if ($build->getAttribute('status') == 'building') { - // Remove the build - $orchestration->remove('build-stage-' . $deployment->getAttribute('buildId'), true); - Console::info('Removed build for deployment ' . $deployment['$id']); - } + if ($build->getAttribute('status') == 'building') { + // Remove the build + $orchestration->remove('build-stage-' . $deployment->getAttribute('buildId'), true); + Console::info('Removed build for deployment ' . $deployment['$id']); } - - // Remove the container of the deployment - $orchestration->remove('appwrite-function-' . $deployment['$id'], true); - Console::info('Removed container for deployment ' . $deployment['$id']); - } catch (Exception $e) { - // Do nothing, we don't care that much if it fails } - } catch (Exception $e) { - logError($e, "cleanupFunction"); - $orchestrationPool->put($orchestration); - return $response->json(['error' => $e->getMessage()]); + // Remove the container of the deployment + $orchestration->remove('appwrite-function-' . $deployment['$id'], true); + Console::info('Removed container for deployment ' . $deployment['$id']); + } catch (Throwable $th) { + $orchestrationPool->put($orchestration); + throw $th; } $orchestrationPool->put($orchestration); @@ -1074,7 +1055,7 @@ App::post('/v1/deployment') $runtime = $runtimes[$function->getAttribute('runtime')] ?? null; if (\is_null($runtime)) { - throw new Exception('Runtime "' . $function->getAttribute('runtime', '') . '" is not supported'); + throw new Exception('Runtime "' . $function->getAttribute('runtime', '') . '" is not supported', 404); } // Create a new build entry @@ -1083,37 +1064,32 @@ App::post('/v1/deployment') if ($deployment->getAttribute('buildId')) { $buildId = $deployment->getAttribute('buildId'); } else { - try { - $dbForProject->createDocument('builds', new Document([ - '$id' => $buildId, - '$read' => (!empty($userId)) ? ['user:' . $userId] : [], - '$write' => ['role:all'], - 'dateCreated' => time(), - 'status' => 'processing', - 'runtime' => $function->getAttribute('runtime'), - 'outputPath' => '', - 'source' => $deployment->getAttribute('path'), - 'sourceType' => Storage::DEVICE_LOCAL, - 'stdout' => '', - 'stderr' => '', - 'time' => 0, - 'vars' => [ - 'ENTRYPOINT_NAME' => $deployment->getAttribute('entrypoint'), - 'APPWRITE_FUNCTION_ID' => $function->getId(), - 'APPWRITE_FUNCTION_NAME' => $function->getAttribute('name', ''), - 'APPWRITE_FUNCTION_RUNTIME_NAME' => $runtime['name'], - 'APPWRITE_FUNCTION_RUNTIME_VERSION' => $runtime['version'], - 'APPWRITE_FUNCTION_PROJECT_ID' => $projectID, - ] - ])); + $dbForProject->createDocument('builds', new Document([ + '$id' => $buildId, + '$read' => (!empty($userId)) ? ['user:' . $userId] : [], + '$write' => ['role:all'], + 'dateCreated' => time(), + 'status' => 'processing', + 'runtime' => $function->getAttribute('runtime'), + 'outputPath' => '', + 'source' => $deployment->getAttribute('path'), + 'sourceType' => Storage::DEVICE_LOCAL, + 'stdout' => '', + 'stderr' => '', + 'time' => 0, + 'vars' => [ + 'ENTRYPOINT_NAME' => $deployment->getAttribute('entrypoint'), + 'APPWRITE_FUNCTION_ID' => $function->getId(), + 'APPWRITE_FUNCTION_NAME' => $function->getAttribute('name', ''), + 'APPWRITE_FUNCTION_RUNTIME_NAME' => $runtime['name'], + 'APPWRITE_FUNCTION_RUNTIME_VERSION' => $runtime['version'], + 'APPWRITE_FUNCTION_PROJECT_ID' => $projectID, + ] + ])); - $deployment->setAttribute('buildId', $buildId); + $deployment->setAttribute('buildId', $buildId); - $dbForProject->updateDocument('deployments', $deployment->getId(), $deployment); - } catch (\Throwable $th) { - var_dump($deployment->getArrayCopy()); - throw $th; - } + $dbForProject->updateDocument('deployments', $deployment->getId(), $deployment); } // Build Code @@ -1156,6 +1132,9 @@ App::post('/v1/deployment') // Deploy Runtime Server createRuntimeServer($functionId, $projectID, $deploymentId, $dbForProject); } catch (\Throwable $th) { + $register->get('dbPool')->put($db); + $register->get('redisPool')->put($redis); + throw $th; } finally { $register->get('dbPool')->put($db); $register->get('redisPool')->put($redis); @@ -1188,41 +1167,31 @@ App::post('/v1/build/:buildId') // Start a Build ->inject('dbForProject') ->inject('projectID') ->action(function (string $buildId, Response $response, Database $dbForProject, string $projectID) { - try { - // Get build document - $build = $dbForProject->getDocument('builds', $buildId); + // Get build document + $build = $dbForProject->getDocument('builds', $buildId); - // Check if build exists - if ($build->isEmpty()) { - throw new Exception('Build not found', 404); - } - - // Check if build is already running - if ($build->getAttribute('status') === 'running') { - throw new Exception('Build is already running', 409); - } - - // Check if build is already finished - if ($build->getAttribute('status') === 'finished') { - throw new Exception('Build is already finished', 409); - } - - go(function () use ($buildId, $dbForProject, $projectID) { - // Build Code - runBuildStage($buildId, $projectID, $dbForProject); - }); - - // return success - return $response->json(['success' => true]); - } catch (Exception $e) { - logError($e, "buildEndpoint"); - - $response - ->addHeader('Cache-Control', 'no-cache, no-store, must-revalidate') - ->addHeader('Expires', '0') - ->addHeader('Pragma', 'no-cache') - ->json(['error' => $e->getMessage()]); + // Check if build exists + if ($build->isEmpty()) { + throw new Exception('Build not found', 404); } + + // Check if build is already running + if ($build->getAttribute('status') === 'running') { + throw new Exception('Build is already running', 409); + } + + // Check if build is already finished + if ($build->getAttribute('status') === 'finished') { + throw new Exception('Build is already finished', 409); + } + + go(function () use ($buildId, $dbForProject, $projectID) { + // Build Code + runBuildStage($buildId, $projectID, $dbForProject); + }); + + // return success + return $response->json(['success' => true]); }); App::setMode(App::MODE_TYPE_PRODUCTION); // Define Mode From 7d3f992a9f1b494dcae5d3e449ed5c978b400aed Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 27 Jan 2022 09:18:48 +0000 Subject: [PATCH 2/8] Add status codes --- app/executor.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/executor.php b/app/executor.php index 1c7d49f84..a509ba1ef 100644 --- a/app/executor.php +++ b/app/executor.php @@ -1026,6 +1026,7 @@ App::post('/v1/cleanup/deployment') } $orchestrationPool->put($orchestration); + $response->setStatusCode(Response::STATUS_CODE_OK); return $response->json(['success' => true]); }); @@ -1191,6 +1192,7 @@ App::post('/v1/build/:buildId') // Start a Build }); // return success + $response->setStatusCode(Response::STATUS_CODE_OK); return $response->json(['success' => true]); }); From d8a9b7e933f294993d4e5d6b2d589b953fc06c44 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 27 Jan 2022 09:27:57 +0000 Subject: [PATCH 3/8] More cleaning up --- app/executor.php | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/app/executor.php b/app/executor.php index a509ba1ef..9b22be1f4 100644 --- a/app/executor.php +++ b/app/executor.php @@ -25,7 +25,6 @@ use Utopia\Database\Query; use Utopia\Database\Validator\Authorization; use Utopia\Database\Validator\UID; use Utopia\Logger\Log; -use Utopia\Orchestration\Adapter\DockerAPI; use Utopia\Orchestration\Adapter\DockerCLI; use Utopia\Orchestration\Orchestration; use Utopia\Registry\Registry; @@ -931,14 +930,15 @@ App::post('/v1/execute') // Define Route function (string $trigger, string $projectId, string $executionId, string $functionId, string $event, string $eventData, string $data, array $webhooks, string $userId, string $jwt, Response $response, Database $dbForProject) { $data = execute($trigger, $projectId, $executionId, $functionId, $dbForProject, $event, $eventData, $data, $webhooks, $userId, $jwt); - $response->setStatusCode(Response::STATUS_CODE_OK); - $response->json($data); + return $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->json($data); } ); // Cleanup Endpoints used internally by appwrite when a function or deployment gets deleted to also clean up their containers App::post('/v1/cleanup/function') - ->param('functionId', '', new UID()) + ->param('functionId', '', new UID(), 'The FunctionID to cleanup') ->inject('response') ->inject('dbForProject') ->action( @@ -958,8 +958,9 @@ App::post('/v1/cleanup/function') // If amount is 0 then we simply return true if (count($results) === 0) { - $response->setStatusCode(Response::STATUS_CODE_OK); - return $response->json(['success' => true]); + return $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->json(['success' => true]); } // Delete the containers of all deployments @@ -979,8 +980,9 @@ App::post('/v1/cleanup/function') Console::info('Removed container for deployment ' . $deployment['$id']); } - $response->setStatusCode(Response::STATUS_CODE_OK); - return $response->json(['success' => true]); + return $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->json(['success' => true]); } catch (Throwable $th) { $orchestrationPool->put($orchestration); throw $th; @@ -1026,8 +1028,9 @@ App::post('/v1/cleanup/deployment') } $orchestrationPool->put($orchestration); - $response->setStatusCode(Response::STATUS_CODE_OK); - return $response->json(['success' => true]); + return $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->json(['success' => true]); }); App::post('/v1/deployment') @@ -1149,17 +1152,6 @@ App::post('/v1/deployment') $response->dynamic($function, Response::MODEL_FUNCTION); }); -App::get('/v1/') - ->inject('response') - ->action( - function (Response $response) { - $response - ->addHeader('Cache-Control', 'no-cache, no-store, must-revalidate') - ->addHeader('Expires', '0') - ->addHeader('Pragma', 'no-cache') - ->json(['status' => 'online']); - } - ); // Build Endpoints App::post('/v1/build/:buildId') // Start a Build @@ -1192,8 +1184,9 @@ App::post('/v1/build/:buildId') // Start a Build }); // return success - $response->setStatusCode(Response::STATUS_CODE_OK); - return $response->json(['success' => true]); + return $response + ->setStatusCode(Response::STATUS_CODE_OK) + ->json(['success' => true]); }); App::setMode(App::MODE_TYPE_PRODUCTION); // Define Mode From fc6e7739ded3b71d9b062245ff0c8a56af66c763 Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Mon, 31 Jan 2022 14:30:55 +0400 Subject: [PATCH 4/8] feat: review comments --- app/executor.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/executor.php b/app/executor.php index c78ef63d5..d280f408e 100644 --- a/app/executor.php +++ b/app/executor.php @@ -936,7 +936,6 @@ App::post('/v1/functions/:functionId/executions') ->action( function (string $trigger, string $projectId, string $executionId, string $functionId, string $event, string $eventData, string $data, array $webhooks, string $userId, string $jwt, Response $response, Database $dbForProject) { $data = execute($trigger, $projectId, $executionId, $functionId, $dbForProject, $event, $eventData, $data, $webhooks, $userId, $jwt); - $response ->setStatusCode(Response::STATUS_CODE_OK) ->json($data); @@ -950,9 +949,10 @@ App::delete('/v1/functions/:functionId') ->inject('dbForProject') ->action( function (string $functionId, Response $response, Database $dbForProject) use ($orchestrationPool) { - /** @var Orchestration $orchestration */ - $orchestration = $orchestrationPool->get(); try { + /** @var Orchestration $orchestration */ + $orchestration = $orchestrationPool->get(); + // Get function document $function = $dbForProject->getDocument('functions', $functionId); @@ -1037,9 +1037,10 @@ App::delete('/v1/deployments/:deploymentId') ->inject('response') ->inject('dbForProject') ->action(function (string $deploymentId, Response $response, Database $dbForProject) use ($orchestrationPool) { - /** @var Orchestration $orchestration */ - $orchestration = $orchestrationPool->get(); try { + /** @var Orchestration $orchestration */ + $orchestration = $orchestrationPool->get(); + // Get deployment document $deployment = $dbForProject->getDocument('deployments', $deploymentId); From 6b77d62e4b700b23258409f59899d676e502e4ac Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 31 Jan 2022 10:46:52 +0000 Subject: [PATCH 5/8] Update app/executor.php Co-authored-by: Christy Jacob --- app/executor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/executor.php b/app/executor.php index d280f408e..e15a505f5 100644 --- a/app/executor.php +++ b/app/executor.php @@ -1069,9 +1069,9 @@ App::delete('/v1/deployments/:deploymentId') } $orchestrationPool->put($orchestration); - return $response + $response ->setStatusCode(Response::STATUS_CODE_OK) - ->json(['success' => true]); + ->send(); }); App::post('/v1/functions/:functionId/deployments/:deploymentId/builds/:buildId') From 663ce27db4f631d7a33a68c484395a803d8d93ad Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 31 Jan 2022 10:47:00 +0000 Subject: [PATCH 6/8] Update app/executor.php Co-authored-by: Christy Jacob --- app/executor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/executor.php b/app/executor.php index e15a505f5..4e3058ebc 100644 --- a/app/executor.php +++ b/app/executor.php @@ -987,9 +987,9 @@ App::delete('/v1/functions/:functionId') Console::info('Removed container for deployment ' . $deployment['$id']); } - return $response + $response ->setStatusCode(Response::STATUS_CODE_OK) - ->json(['success' => true]); + ->send(); } catch (Throwable $th) { $orchestrationPool->put($orchestration); throw $th; From 30a567058ab859dbca2c939959fbbcc79f0f1d71 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 31 Jan 2022 10:47:04 +0000 Subject: [PATCH 7/8] Update app/executor.php Co-authored-by: Christy Jacob --- app/executor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/executor.php b/app/executor.php index 4e3058ebc..bc5ddc9e6 100644 --- a/app/executor.php +++ b/app/executor.php @@ -965,9 +965,9 @@ App::delete('/v1/functions/:functionId') // If amount is 0 then we simply return true if (count($results) === 0) { - return $response + $response ->setStatusCode(Response::STATUS_CODE_OK) - ->json(['success' => true]); + ->send(); } // Delete the containers of all deployments From a47211e7eeb953e85bd6a5f88d50d6b166c6dde5 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 31 Jan 2022 10:57:57 +0000 Subject: [PATCH 8/8] Update executor.php --- app/executor.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/executor.php b/app/executor.php index 9b22be1f4..d16cb4ce0 100644 --- a/app/executor.php +++ b/app/executor.php @@ -363,7 +363,7 @@ function execute(string $trigger, string $projectId, string $executionId, string } - if ($build->getAttribute('status') == 'building') { + if ($build->getAttribute('status') === 'building') { $execution ->setAttribute('status', 'failed') @@ -632,7 +632,7 @@ function execute(string $trigger, string $projectId, string $executionId, string roles: $target['roles'] ); - if (App::getEnv('_APP_USAGE_STATS', 'enabled') == 'enabled') { + if (App::getEnv('_APP_USAGE_STATS', 'enabled') === 'enabled') { $statsd = $register->get('statsd'); $usage = new Stats($statsd); @@ -652,7 +652,7 @@ function execute(string $trigger, string $projectId, string $executionId, string return [ 'status' => $functionStatus, - 'response' => ($functionStatus !== 'completed') ? $stderr : $stdout, + 'response' => ($functionStatus !=== 'completed') ? $stderr : $stdout, 'time' => $executionTime ]; }; @@ -868,7 +868,7 @@ function runBuildStage(string $buildId, string $projectID): Document } } - if ($buildStdout == '') { + if ($buildStdout === '') { $buildStdout = 'Build Successful!'; } @@ -1012,7 +1012,7 @@ App::post('/v1/cleanup/deployment') if ($deployment->getAttribute('buildId')) { $build = $dbForProject->getDocument('builds', $deployment->getAttribute('buildId')); - if ($build->getAttribute('status') == 'building') { + if ($build->getAttribute('status') === 'building') { // Remove the build $orchestration->remove('build-stage-' . $deployment->getAttribute('buildId'), true); Console::info('Removed build for deployment ' . $deployment['$id']);