Add deployment environment API endpoint #2412
Conversation
dotNomad
requested review from
kgartland-rstudio,
sagerb and
marcosnav
as code owners
| @@ -166,6 +166,10 @@ func RouterHandlerFunc(base util.AbsolutePath, lister accounts.AccountList, log | |||
| r.Handle(ToPath("deployments", "{name}"), DeleteDeploymentHandlerFunc(base, log)). | |||
| Methods(http.MethodDelete) | |||
|
|
|||
| // GET /api/deployments/$NAME/environment | |||
| r.Handle(ToPath("deployments", "{name}", "environment"), GetDeploymentEnvironmentHandlerFunc(base, log, lister)). | |||
There was a problem hiding this comment.
Unsure about the name here. Maybe we should be more direct about this being the Connect Content environment?
This is on the deployment resource which implies this is the content, and not the configuration's environment.
There was a problem hiding this comment.
I think it makes sense as you have it 👍
| // If the deployment file is in error return a 400 | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| w.Write([]byte(fmt.Sprintf("deployment %s is invalid: %s", name, err))) | ||
| return |
There was a problem hiding this comment.
Many of these error responses can be sent as specific agent errors, like in post_deployment.go. I'm guessing those changes are scoped to a future PR, when bridging the frontend, but wanted to confirm.
There was a problem hiding this comment.
That is exactly right. I kept it generic for now, and figured we would add agent errors when they were needed by the agent
| @@ -166,6 +166,10 @@ func RouterHandlerFunc(base util.AbsolutePath, lister accounts.AccountList, log | |||
| r.Handle(ToPath("deployments", "{name}"), DeleteDeploymentHandlerFunc(base, log)). | |||
| Methods(http.MethodDelete) | |||
|
|
|||
| // GET /api/deployments/$NAME/environment | |||
| r.Handle(ToPath("deployments", "{name}", "environment"), GetDeploymentEnvironmentHandlerFunc(base, log, lister)). | |||
There was a problem hiding this comment.
I think it makes sense as you have it 👍
This PR adds a new endpoint for our binary
GET /api/deployments/$NAME/environmentwhich will either:I kept all of the errors bubbling up since the frontend can handle this by ignoring them for the most part and leaving better Connect Content error handling to other, more pointed features.
The plan is to capture these errors and assume that the environment is
[]if something goes awry.Intent
Part of #2365
Type of Change
Approach
The approach was to fetch everything that is necessary to call the Connect API and fail on the different cases that can happen before calling the API.
Then if we call the API pass through any errors.
Automated Tests
There are automated tests for the new endpoint.
Directions for Reviewers
The API can be tested directly using
just run. There are tests for the various error states:404400400400InternalErrorsfor unexpected cases