Skip to content
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

Add workflow run entity #6622

Merged
merged 6 commits into from
Aug 14, 2024
Merged

Add workflow run entity #6622

merged 6 commits into from
Aug 14, 2024

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Aug 14, 2024

  • create a workflow run every time a workflow is triggered in not_started status. This status will be helpful later for once workflows will be scheduled
  • update run status once workflow starts running
  • complete status once the workflow finished running
  • add a failed status if an error occurs

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request introduces a new WorkflowRunWorkspaceEntity to track the execution of workflows, implementing functionality to create, update, and manage workflow run statuses throughout the workflow execution process.

  • Added WorkflowRunWorkspaceEntity in workflow-run.workspace-entity.ts with fields for start/end times and status
  • Implemented WorkflowStatusService in workflow-status.service.ts to manage workflow run lifecycle (create, start, end)
  • Updated WorkflowRunnerJob in workflow-runner.job.ts to use WorkflowStatusService for tracking run progress
  • Modified WorkflowEventTriggerJob in workflow-event-trigger.job.ts to create a workflow run before execution
  • Introduced WorkflowStatusException in workflow-status.exception.ts for handling specific workflow status errors

13 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings

icon: 'IconHistory',
})
@WorkspaceIsNullable()
status: WorkflowRunStatus;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider making status non-nullable to ensure it always has a valid value

Comment on lines 26 to 34
const onGoingWorkflowRuns = (
await workflowRunDataSource.findBy({
workflowVersionId,
})
).filter((workflow) =>
[WorkflowRunStatus.NOT_STARTED, WorkflowRunStatus.RUNNING].includes(
workflow.status,
),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using a database query to filter ongoing workflow runs instead of fetching all and filtering in memory. This approach may not scale well with a large number of workflow runs.


if (onGoingWorkflowRuns.length > 0) {
throw new WorkflowStatusException(
'There is already an on going workflow run',
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: 'on going' should be 'ongoing'

Suggested change
'There is already an on going workflow run',
'There is already an ongoing workflow run',

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

LGTM!

@thomtrp thomtrp merged commit 9e7714e into main Aug 14, 2024
6 checks passed
@thomtrp thomtrp deleted the tt-follow-workflow-status branch August 14, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants