self review PR

freiheit-com · Oct 14, 2024 · 5f2773e · 5f2773e
1 parent a42bc74
commit 5f2773e
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 12 deletions.
diff --git a/pkg/api/v1/api.proto b/pkg/api/v1/api.proto
@@ -389,8 +389,7 @@ service EnvironmentService {
 message GetChangedAppsRequest {}
 
 message GetChangedAppsResponse {
-  repeated string full_list_apps = 1;
-  repeated string changed_apps = 2;
+  repeated string changed_apps = 1;
 }
 
 message GetAppDetailsRequest {

diff --git a/services/cd-service/pkg/service/overview.go b/services/cd-service/pkg/service/overview.go
@@ -337,7 +337,6 @@ func (o *OverviewServiceServer) StreamOverview(in *api.GetOverviewRequest,
 			return nil
 		case <-ch:
 			ov := o.response.Load().(*api.GetOverviewResponse)
-			//o.changedApps.Load()
 			if err := stream.Send(ov); err != nil {
 				// if we don't log this here, the details will be lost - so this is an exception to the rule "either return an error or log it".
 				// for example if there's an invalid encoding, grpc will just give a generic error like
@@ -364,16 +363,11 @@ func (o *OverviewServiceServer) StreamChangedApps(in *api.GetChangedAppsRequest,
 			return nil
 		case changedApps := <-ch:
 			ov := &api.GetChangedAppsResponse{
-				ChangedApps:  changedApps,
-				FullListApps: []string{},
+				ChangedApps: changedApps,
 			}
 			logger.FromContext(stream.Context()).Sugar().Infof("Got changes apps: '%v'\n", changedApps)
 			if err := stream.Send(ov); err != nil {
-				// if we don't log this here, the details will be lost - so this is an exception to the rule "either return an error or log it".
-				// for example if there's an invalid encoding, grpc will just give a generic error like
-				// "error while marshaling: string field contains invalid UTF-8"
-				// but it won't tell us which field has the issue. This is then very hard to debug further.
-				logger.FromContext(stream.Context()).Error("error sending overview response:", zap.Error(err), zap.String("overview", fmt.Sprintf("%+v", ov)))
+				logger.FromContext(stream.Context()).Error("error sending changed apps  response:", zap.Error(err), zap.String("changedApps", fmt.Sprintf("%+v", ov)))
 				return err
 			}
 

diff --git a/services/rollout-service/pkg/argo/argo.go b/services/rollout-service/pkg/argo/argo.go
@@ -147,8 +147,8 @@ func (a *ArgoAppProcessor) Consume(ctx context.Context, hlth *setup.HealthReport
 }
 
 type ArgoOverview struct {
-	AppDetails map[string]*api.GetAppDetailsResponse
-	Overview   *api.GetOverviewResponse
+	AppDetails map[string]*api.GetAppDetailsResponse //Map from appName to app Details. Gets filled with information based on what apps have changed.
+	Overview   *api.GetOverviewResponse              //Standard overview. Only information regarding environments should be retrieved from this overview.
 }
 
 func (a *ArgoAppProcessor) CreateOrUpdateApp(ctx context.Context, overview *api.GetOverviewResponse, appName, team string, env *api.Environment, appsKnownToArgo map[string]*v1alpha1.Application) {

diff --git a/services/rollout-service/pkg/versions/versions_test.go b/services/rollout-service/pkg/versions/versions_test.go
@@ -911,6 +911,8 @@ func assertStep(t *testing.T, i int, s step, vp *mockVersionEventProcessor, hs *
 	if hs.IsReady("versions") != s.ExpectReady {
 		t.Errorf("wrong readyness in step %d, expected %t but got %t", i, s.ExpectReady, hs.IsReady("versions"))
 	}
+
+	//Sort this to avoid flakeyness based on order
 	sort.Slice(vp.events, func(i, j int) bool {
 		return vp.events[i].Environment < vp.events[j].Environment
 	})