Implement script runner microservice#7896
Conversation
Change class name of script microservice runner Add config for microservice ruuner Add callback route Remove send first response in TestScript Add version script in env file Move callback to api Implement script microservice in runscripttask.php Remove config of script-runners Adding type column to script executor table Change int for bool to sync variable fix composer.json file Fix api environment variables Fix script language capitalize Fix null version field Change output name in json callback Remove json_decode in output response Add script executor enabled flag Rollback _fonts.scss file
| { | ||
| $scriptMicroserviceService = new ScriptMicroserviceService(); | ||
| $scriptMicroserviceService->handle($request); | ||
| } |
There was a problem hiding this comment.
Hey @gusys should we add error handling to catch any potential exceptions from the microservice execution?
nolanpro
left a comment
There was a problem hiding this comment.
@gusys Looks good, please see my comments.
There are a few failing unit tests that need to be fixed.
I think we should have clear instructions in the README for how developers can use the service.
For most day-to-day development, the engineer’s local environment should connect to the hosted microservice development cluster.
However, when changes need to be made to microservice or custom executors, the engineer should be able to switch use their local cluster (environment variable) and easily build and execute images locally. The local development cluster should closely resemble the production instance. This could be done through Minikube, k3s, docker compose, etc. Whichever one you choose, I think the local configuration file and instructions should be included in core.
| * @param array $config | ||
| */ | ||
| public function runScript(array $data, array $config, $tokenId = '', $timeout = null) | ||
| public function runScript(array $data, array $config, $tokenId = '', $timeout = null, $sync = 1, $metadata = []) |
There was a problem hiding this comment.
@gusys could you change $sync = 1 to bool $sync = true? You can change it in ScriptMicroserviceRunner if you need to send it as an integer 'sync' => $sync ? 1 : 0,
There are a few other places we call runScript, like RunServiceTask.php. Do those need to be updated?
| 'token_id' => $this->tokenId, | ||
| ], | ||
| ]; | ||
| $response = $script->runScript($data, $configuration, $token->getId(), $errorHandling->timeout(), 0, $metadata); |
There was a problem hiding this comment.
@gusys if this is an asynchronous execution, what endpoint are we using for the callback? Is it /scripts/microservice/execution? Is that also the callback when executed from TestScript.php?
routes/api.php
Outdated
| }); | ||
| }); | ||
|
|
||
| Route::prefix('api')->name('api.')->group(function () { |
There was a problem hiding this comment.
@gusys looks like this endpoint has no security. How will this be protected?
|
Change class name of script microservice runner
Add config for microservice ruuner
Add callback route
Remove send first response in TestScript
Add version script in env file
Move callback to api
Implement script microservice in runscripttask.php
Remove config of script-runners
Adding type column to script executor table
Change int for bool to sync variable
fix composer.json file
Fix api environment variables
Fix script language capitalize
Fix null version field
Change output name in json callback
Remove json_decode in output response
Add script executor enabled flag
Rollback _fonts.scss file
Issue & Reproduction Steps
Describe the issue this ticket solves and describe how to reproduce the issue (please attach any fixtures used to reproduce the issue).
Solution
How to Test
Describe how to test that this solution works.
Related Tickets & Packages
Code Review Checklist