fix(reviewapp): Prevent non-existent app build #61
Conversation
ipmb
left a comment
ipmb
left a comment
There was a problem hiding this comment.
GetReviewApps is going to be slow. You shouldn't need to iterate over all the values to see if this one exists, just try to read the specific parameter for this app.
app/app.go
Outdated
| if !a.Pipeline { | ||
| return false, fmt.Errorf("%s is not a pipeline and cannot have review apps", a.Name) | ||
| } | ||
| _, err := SsmParameter(a.Session, fmt.Sprintf("/apppack/pipelines/%s/review-apps/pr/%s", a.Name, *a.ReviewApp)) |
There was a problem hiding this comment.
You might actually need to check the value of this. It should be JSON with a status key that needs a value of created. See https://github.com/apppackio/apppack/blob/main/cmd/reviewapps.go#L46
There was a problem hiding this comment.
Ok, I'll take a look.
There was a problem hiding this comment.
I had a look, and it seems like that the parameter in this case will only exist if the reviewapp exists. Isn't that correct? Since we only are concerned with review app existing, why would it matter for it to be in the created status?
There was a problem hiding this comment.
The parameter gets created when the PR is created at GitHub. That functionality is in the builder here, https://github.com/apppackio/apppack-codebuild-image/blob/main/builder/build/prebuild.go#L301-L304
In that case, the parameter will exist, but the review app will not.
CuriousLearner
deleted the
prevent-build-command-for-non-existent-review-app
branch
Fixes #44
Presents error on
&
if the review app does not exist.