You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/08/11 07:06:47 UTC

[GitHub] [yunikorn-core] manirajv06 commented on a diff in pull request #428: [YUNIKORN-1276] REST API for specific application

manirajv06 commented on code in PR #428:
URL: https://github.com/apache/yunikorn-core/pull/428#discussion_r943158875


##########
pkg/webservice/handlers.go:
##########
@@ -677,6 +677,62 @@ func getQueueApplications(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+func getApplication(w http.ResponseWriter, r *http.Request) {
+	vars := mux.Vars(r)
+	writeHeaders(w)
+	partition, partitionExists := vars["partition"]
+	if !partitionExists {
+		buildJSONErrorResponse(w, "Partition is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+		return
+	}
+	queueName, queueNameExists := vars["queue"]
+	if !queueNameExists {
+		buildJSONErrorResponse(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+		return
+	}
+
+	application, applicationExists := vars["application"]
+	if !applicationExists {
+		buildJSONErrorResponse(w, "Application Id is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+		return
+	}
+
+	queueErr := validateQueue(queueName)
+	if queueErr != nil {
+		buildJSONErrorResponse(w, queueErr.Error(), http.StatusBadRequest)
+		return
+	}
+	if len(vars) != 3 {
+		buildJSONErrorResponse(w, "Incorrect URL path. Please check the usage documentation", http.StatusBadRequest)

Review Comment:
   We followed the same order while redesigned the whole REST API's. But then realised that doing 2 before 1 gives clear and concise err message by doing simple check on request parameters. For example, Pointing out specific param is missing is very clear instead of saying generic incorrect url path or something like that. We had discussion around this as part of design and through jira comments as well. Finally, we landed up in the current order. If you still think, may be we can handle this new jira as this require changes in all API's.



-- 
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