[GitHub] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #372: [YUNIKORN-961] Expose build and version information in a REST call

[GitBox <gi...@apache.org>]

wilfred-s commented on a change in pull request #372:
URL: https://github.com/apache/incubator-yunikorn-core/pull/372#discussion_r813582862



##########
File path: pkg/webservice/handlers.go
##########
@@ -200,25 +193,13 @@ func buildJSONErrorResponse(w http.ResponseWriter, detail string, code int) {
 	}
 }
 
-func getBuildInfoJSON(rmInfo *scheduler.RMInformation) *dao.BuildInfo {
-	rmInfos := schedulerContext.GetRMInfoMapClone()
-	rmBuildInfos := make([]dao.RMBuildInfo, 0, len(rmInfos))
-
-	for _, rmInfo := range rmInfos {
-		rmInformation := dao.RMBuildInfo{
-			RMBuildInformation: rmInfo.RMBuildInformation,
-		}
-		rmBuildInfos = append(rmBuildInfos, rmInformation)
-	}
-
-	return &dao.BuildInfo{
-		ScheduleStartDate: dao.ScheduleStartDate,
-		RMBuildInfo:       rmBuildInfos,
-	}
-}
-
 func getClusterJSON(partition *scheduler.PartitionContext) *dao.ClusterDAOInfo {
 	clusterInfo := &dao.ClusterDAOInfo{}
+	clusterInfo.ScheduleStartDate = dao.ScheduleStartDate

Review comment:
       change to schedulerContext.GetStartTime and format it nicely here.

##########
File path: pkg/webservice/dao/cluster_info.go
##########
@@ -18,18 +18,22 @@
 package dao
 
 type ClusterDAOInfo struct {
-	PartitionName         string `json:"partition"`
-	ClusterName           string `json:"clusterName"`
-	TotalApplications     string `json:"totalApplications"`
-	FailedApplications    string `json:"failedApplications"`
-	PendingApplications   string `json:"pendingApplications"`
-	RunningApplications   string `json:"runningApplications"`
-	CompletedApplications string `json:"completedApplications"`
-	TotalContainers       string `json:"totalContainers"`
-	FailedContainers      string `json:"failedContainers"`
-	PendingContainers     string `json:"pendingContainers"`
-	RunningContainers     string `json:"runningContainers"`
-	ActiveNodes           string `json:"activeNodes"`
-	TotalNodes            string `json:"totalNodes"`
-	FailedNodes           string `json:"failedNodes"`
+	ScheduleStartDate     string              `json:"scheduleStartDate"`
+	RMBuildInformation    []map[string]string `json:"rmBuildInformation"`
+	PartitionName         string              `json:"partition"`
+	ClusterName           string              `json:"clusterName"`
+	TotalApplications     string              `json:"totalApplications"`
+	FailedApplications    string              `json:"failedApplications"`
+	PendingApplications   string              `json:"pendingApplications"`
+	RunningApplications   string              `json:"runningApplications"`
+	CompletedApplications string              `json:"completedApplications"`
+	TotalContainers       string              `json:"totalContainers"`
+	FailedContainers      string              `json:"failedContainers"`
+	PendingContainers     string              `json:"pendingContainers"`
+	RunningContainers     string              `json:"runningContainers"`
+	ActiveNodes           string              `json:"activeNodes"`
+	TotalNodes            string              `json:"totalNodes"`
+	FailedNodes           string              `json:"failedNodes"`
 }
+
+var ScheduleStartDate string

Review comment:
       Move this as a variable of type _time.Time_ with the name startTime ​into the `ClusterContext` object.
   Set it as part of the `NewClusterContext` and `newClusterContext` calls.
   Add a locked method to the clustercontext `GetStartTime() time.Time` to retrieve the value. 
   
   That should also remove the dao dependency from the entrypoint which is much cleaner




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org