Implement planpreview handler #2143
Conversation
pipecd-bot
left a comment
pipecd-bot
left a comment
There was a problem hiding this comment.
Some issues were detected while linting go source files in your changes.
|
Code coverage for golang is
|
|
Code coverage for golang is
|
|
I know it's a bit tedious and likely to have no sense, but it would be helpful to review if you could write down a brief summary or where you want us to look over if there is. No need to do it every time, but it's gonna make for a good discussion if you could give it a try as much as possible 😃 |
| if err != nil { | ||
| return nil, fmt.Errorf("failed to clone git repository %s", cmd.RepositoryId) | ||
| } | ||
| defer repo.Clean() |
There was a problem hiding this comment.
I didn't know there is such a helper.
|
Cool. there you go! |
Sure. I have just updated the PR body to explain more about this PR. |
|
Thank you! |
pkg/app/piped/planpreview/handler.go
Outdated
| workerNum: 3, | ||
| commandQueueBufferSize: 10, | ||
| commandCheckInterval: 5 * time.Second, | ||
| commandHandleTimeout: 5 * time.Minute, |
There was a problem hiding this comment.
nit: to be more clear they should be defined as const like default~, some other places don't be so though.
There was a problem hiding this comment.
Agree, make them as const variables of this planpreview package is LGTM 👍
|
The following ISSUES will be created once got merged. If you want me to skip creating the issue, you can use Details1. Implement planpreview builder.This was created by todo plugin since "TODO:" was found in b7ea8c3 when #2143 was merged. cc: @nghialv.2. Add a new field to show why command was failed.This was created by todo plugin since "TODO:" was found in b7ea8c3 when #2143 was merged. cc: @nghialv. |
|
Nice 🚀 |
|
This pull request is APPROVED by Approvers can cancel the approval by writing |
What this PR does / why we need it:
This PR continues implementing the PlanPreview feature described in this RFC.
In this PR, I have added a new
handlercomponent that will run when piped started up.This handler periodically fetches all un-handled BuildPlanPreview commands and split them into a pool of workers.
Each worker handles command serially by making a
builderinstance to build appropriate PlanPreview results for the command.Which are the main parts that should be reviewed:
handlerstruct and its behaviour defined insideplanpreviewpackageWhich issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: