Skip to content

Implement script runner microservice #7896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 24, 2025

Conversation

gusys
Copy link
Contributor

@gusys gusys commented Jan 14, 2025

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

  • List the changes you've introduced to solve the issue.

How to Test

Describe how to test that this solution works.

Related Tickets & Packages

  • Link to any related FOUR tickets, PRDs, or packages

Code Review Checklist

  • I have pulled this code locally and tested it on my instance, along with any associated packages.
  • This code adheres to ProcessMaker Coding Guidelines.
  • This code includes a unit test or an E2E test that tests its functionality, or is covered by an existing test.
  • This solution fixes the bug reported in the original ticket.
  • This solution does not alter the expected output of a component in a way that would break existing Processes.
  • This solution does not implement any breaking changes that would invalidate documentation or cause existing Processes to fail.
  • This solution has been tested with enterprise packages that rely on its functionality and does not introduce bugs in those packages.
  • This code does not duplicate functionality that already exists in the framework or in ProcessMaker.
  • This ticket conforms to the PRD associated with this part of ProcessMaker.

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);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @gusys should we add error handling to catch any potential exceptions from the microservice execution?

Copy link
Contributor

@nolanpro nolanpro left a comment

Choose a reason for hiding this comment

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

@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.

@@ -149,7 +149,7 @@ public static function rules($existing = null)
* @param array $data
* @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 = [])
Copy link
Contributor

@nolanpro nolanpro Jan 15, 2025

Choose a reason for hiding this comment

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

@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);
Copy link
Contributor

@nolanpro nolanpro Jan 15, 2025

Choose a reason for hiding this comment

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

@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
@@ -411,3 +411,7 @@
Route::get('devlink/{devLink}', [DevLinkController::class, 'show'])->name('devlink.show');
});
});

Route::prefix('api')->name('api.')->group(function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

@gusys looks like this endpoint has no security. How will this be protected?

Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@ryancooley ryancooley merged commit fefe5f4 into release-2025-winter Jan 24, 2025
7 of 8 checks passed
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.

5 participants