-
Notifications
You must be signed in to change notification settings - Fork 9
Windmill integration #993
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
Windmill integration #993
Conversation
internal/ent/hooks/scheduled_job.go
Outdated
if mutation.Op() == ent.OpCreate { | ||
if err := createWindmillFlow(ctx, mutation); err != nil { | ||
return nil, fmt.Errorf("failed to create windmill flow: %w", err) | ||
} | ||
} | ||
|
||
return next.Mutate(ctx, mutation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should happen after the next.Mutate, so we can make sure the validation happens correctly and then the flow is created in windmill
if mutation.Op() == ent.OpCreate { | |
if err := createWindmillFlow(ctx, mutation); err != nil { | |
return nil, fmt.Errorf("failed to create windmill flow: %w", err) | |
} | |
} | |
return next.Mutate(ctx, mutation) | |
retVal, err := next.Mutate(ctx, mutation) | |
if err != nil { | |
return err | |
} | |
if mutation.Op() == ent.OpCreate { | |
if err := createWindmillFlow(ctx, mutation); err != nil { | |
return nil, fmt.Errorf("failed to create windmill flow: %w", err) | |
} | |
} | |
return retVal, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work if a user updates the job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this, I do not want to make fields like the windmill path optional - as they aren't. the job creation also does all possible validation asides the context.
let me know if to move still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha, that makes sense - but we would still need to be able to roll this back if the transaction fails so we don't end up with a hanging flow if something causes the db transaction to fail
Co-authored-by: Sarah Funkhouser <[email protected]> Signed-off-by: Lanre Adelowo <[email protected]>
🔧 Configuration Changes DetectedThis PR contains changes that will affect the Helm chart configuration. A draft infrastructure PR has been automatically created to preview these changes: 📋 Draft PR: theopenlane/openlane-infra#100 Changes Preview:
The infrastructure PR will automatically convert from draft to ready for review once this core PR is merged. |
🔧 Configuration Changes DetectedThis PR contains changes that will affect the Helm chart configuration. A draft infrastructure PR has been automatically created to preview these changes: 📋 Draft PR: theopenlane/openlane-infra#100 Changes Preview:🔧 Applying Helm configuration changes... The infrastructure PR will automatically convert from draft to ready for review once this core PR is merged. |
🔧 Configuration Changes DetectedThis PR contains changes that will affect the Helm chart configuration. A draft infrastructure PR has been automatically created to preview these changes: 📋 Draft PR: theopenlane/openlane-infra#111 Changes Preview:🔧 Applying Helm configuration changes... The infrastructure PR will automatically convert from draft to ready for review once this core PR is merged. |
|
Uh oh!
There was an error while loading. Please reload this page.