Skip to content

Persistent Executions Data: Initial Commit#746

Open
jjoderis wants to merge 11 commits into
mainfrom
persistent-deployment-data
Open

Persistent Executions Data: Initial Commit#746
jjoderis wants to merge 11 commits into
mainfrom
persistent-deployment-data

Conversation

@jjoderis
Copy link
Copy Markdown
Contributor

Summary

First parts of our persistent execution data implementation. Added db tables to store information about process deployments and instances and update the data on user input.

Details

Engine

  • the endpoint for starting instances will now return the initial state of the started instance instead of only the id of the instance

Management System

  • added a db table for process deployments that stores information about the deployed version, the user that deployed the process, the engine the process was deployed to and potentially instances that were started from this deployment
  • added a db table for process instances that stores information about the user that started the instance, the engines the instance is running on, and the deployment the instance was started from
  • using the new persistent information to reduce the need for fetching when trying to manage a deployment or instance
  • moved some of the instance management logic from the frontend to the backend by using the new persistently stored data instead of the in memory data we were using before
  • TODO: continuously refetch the deployment and instance data to keep the state in the DB fresh without user interaction

@github-actions

This comment has been minimized.


const data = DeploymentInputSchema.parse(input);

const result = await db.processDeployment.createMany({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does createMany return the created records here? I mean if caller code expects the actual deployment records (instead of just a count), it might be a problem then. So may be we should use createManyAndReturn or multiple create calls wrapped in a transaction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The return value "result" is not used by the calling function "deployProcess" so I removed it.

if (!skipAbilityCheck) {
const { ability } = await getCurrentEnvironment(spaceId);

if (!ability.can('create', 'Execution'))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we also have update ability. If I am not wrong then may be here; for an update operation, 'update' permission should be checked

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Absolutely correct. I changed the permission check to 'udpate'.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown

CLOUDRUN ACTIONS

✅ Successfully created Preview Deployment.

https://pr-746---ms-server-staging-c4f6qdpj7q-ew.a.run.app

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.

2 participants