Create insight data for MTTR #1198
Conversation
|
Failed while validating Kapetanios configuration. Details |
|
This pull request is NOT APPROVED. It will be approved when one of the following conditions is met:
|
|
The golinter build is completed with FAILURE. The build will be triggered again when you push any other commits. Or you can trigger it manually by You can check the build log from here. |
…e-insight-data-for-mttr
|
The following files are not gofmt-ed. By commenting pkg/app/api/grpcapi/web_api_test.go--- pkg/app/api/grpcapi/web_api_test.go.orig
+++ pkg/app/api/grpcapi/web_api_test.go
@@ -391,7 +391,6 @@
}
}
-
func TestCalculateInsightData(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
|
|
/golinter fmt |
|
Code coverage for golang is
|
…e-insight-data-for-mttr
|
Code coverage for golang is
|
|
Code coverage for golang is
|
|
/lgtm |
pkg/app/api/grpcapi/web_api.go
Outdated
| }, | ||
| { | ||
| Field: "ApplicationId", | ||
| Operator: "==", |
There was a problem hiding this comment.
Actually, we can use the "in" operator. With it, you can query a specific field for multiple values in a single query. (Maybe we should clearly define what operator is available.
There was a problem hiding this comment.
OK, fix to use in operator, thanks.
…e-insight-data-for-mttr
|
Code coverage for golang is
|
pkg/app/api/grpcapi/web_api.go
Outdated
|
|
||
| var applicationIDs []string | ||
| if applicationID == "" { | ||
| apps, err := a.applicationStore.ListApplications(ctx, datastore.ListOptions{}) |
There was a problem hiding this comment.
Why do we need to list the apps when the appID was not specified?
Doesn't an empty appID mean all apps of the project?
There was a problem hiding this comment.
Simply my mistake.
Since the original function calculateAverageMTTR was to calculate the MTTR of an application, I got all the appId's and calculated the MTTR by getting the deployment from the DB for each application in a loop to use calculateAverageMTTR around.
I changed calculateAverageMTTR to calculate MTTR from a mix of applications when I received nakabonne review, so I should have noticed that I don't need to get all the appId's here.
Thanks :D
|
Talking with team members and we noticed that there are many other patterns to calcurate MTTR. So, close for now. |
|
Code coverage for golang is
|
What this PR does / why we need it:
Create insight data for MTTR
Which issue(s) this PR fixes:
ref #1142
Does this PR introduce a user-facing change?:
note
This branch is based on create-insight-data-for-change-failure-rate to make the diff easier to understand. So, merge this PR after #1197