wilfred-s commented on code in PR #428:
URL: https://github.com/apache/yunikorn-core/pull/428#discussion_r942162271
##########
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 need to be consistent with these checks. I would expect a standard
ordering:
1. the number of arguments
2. the existence of each argument by name
3. the content of the argument syntax check (if required)
We do not seem to do that consistently in any of the calls. It means we do
too much processing before the eventual failure.
##########
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)
Review Comment:
Need to make this constants or something not add the same text in multiple
places in the handler, same for all error messages for parameter checks.
##########
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)
+ return
+ }
+
+ partitionContext :=
schedulerContext.GetPartitionWithoutClusterID(partition)
+ if partitionContext == nil {
+ buildJSONErrorResponse(w, "Partition not found",
http.StatusBadRequest)
+ return
+ }
+
+ queue := partitionContext.GetQueue(queueName)
+ if queue == nil {
+ buildJSONErrorResponse(w, "Queue not found",
http.StatusBadRequest)
+ return
+ }
+
+ app := queue.GetApplication(application)
+ if app == nil {
+ buildJSONErrorResponse(w, "Application not found",
http.StatusBadRequest)
+ return
+ }
+
+ appDao := make([]*dao.ApplicationDAOInfo, 0, 1)
Review Comment:
Do we really want to return an array of objects? We know we only have 1
application that we're returning.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
