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

Automation Orchestrator refactor. #15546

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

samwho
Copy link
Collaborator

@samwho samwho commented Feb 13, 2025

Description

The motivation for this is a bug in the automation test output when combining a filter step with loop steps: https://linear.app/budibase/issue/BUDI-9015/looped-automation-steps-showing-up-in-test-results-after-a-failed.

In reading the code to try and fix this, I decided to take the plunge and change the way the Orchestrator works in order to make this bug fix easier.

My recommendation for reviewing this is to pull the branch into your local environment and start reading from packages/server/src/threads/automation.ts. That's where the Orchestrator class, the thing that runs automations, lives and where most of the changes are concentrated.

Addresses

Copy link

qa-wolf bot commented Feb 13, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/xl labels Feb 13, 2025
@samwho samwho changed the title Fix automation loop test output 2 Automation Orchestrator refactor. Feb 13, 2025
@@ -1134,7 +1138,7 @@ const automationActions = (store: AutomationStore) => ({
* @returns
*/
shiftBranch: (pathTo: Array<any>, block: AutomationStep, direction = -1) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the block input not be BranchStep instead?

@@ -13,19 +13,17 @@ export interface TriggerOutput {
timestamp?: number
}

export interface AutomationContext extends AutomationResults {
export interface AutomationContext {
trigger: any
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this not be an AutomationTrigger?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
firestorm Data/Infra/Revenue Team size/xl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants