wilfred-s commented on code in PR #465:
URL: https://github.com/apache/yunikorn-core/pull/465#discussion_r1041727306


##########
pkg/webservice/handlers.go:
##########
@@ -746,6 +750,25 @@ func getUsersResourceUsage(w http.ResponseWriter, r 
*http.Request) {
        }
 }
 
+func getUserResourceUsage(w http.ResponseWriter, r *http.Request) {
+       userManager := ugm.GetUserManager()

Review Comment:
   We do not request this until after we have confirmed that we have the 
required parameters



##########
pkg/webservice/handlers_test.go:
##########
@@ -1317,6 +1353,110 @@ func TestSetLoggerLevel(t *testing.T) {
        assert.Equal(t, rr.Code, http.StatusOK)
 }
 
+func TestSpecificUserAndGroupResourceUsage(t *testing.T) {
+       part := setup(t, configDefault, 1)
+       userManager := ugm.GetUserManager()
+       userManager.ClearUserTrackers()
+       userManager.ClearGroupTrackers()
+
+       // add 1 application
+       app := addAppWithUserGroup(t, "app-1", part, "root.default", false, 
security.UserGroup{
+               User:   "testuser",
+               Groups: []string{"testgroup"},
+       })
+       res := &si.Resource{
+               Resources: map[string]*si.Quantity{"vcore": {Value: 1}},
+       }
+       ask := objects.NewAllocationAskFromSI(&si.AllocationAsk{
+               ApplicationID:  "app-1",
+               PartitionName:  part.Name,
+               ResourceAsk:    res,
+               MaxAllocations: 1})
+       err := app.AddAllocationAsk(ask)
+       assert.NilError(t, err, "ask should have been added to app")
+
+       // add an alloc
+       uuid := "uuid-1"
+       allocInfo := objects.NewAllocation(uuid, "node-1", ask)
+       app.AddAllocation(allocInfo)
+       assert.Assert(t, app.IsStarting(), "Application did not return starting 
state after alloc: %s", app.CurrentState())

Review Comment:
   Can we factor this out to a separate function. This is at least repeated 
twice and with the split of the test for user(s) and group(s) handlers it will 
be used many times more.



##########
pkg/webservice/handlers_test.go:
##########
@@ -1358,6 +1376,34 @@ func TestUsersAndGroupsResourceUsage(t *testing.T) {
        assert.Equal(t, usersResourceUsageDao[0].Queues.ResourceUsage.String(),
                
resources.NewResourceFromMap(map[string]resources.Quantity{siCommon.CPU: 
1}).String())
 
+       // Test existed user query
+       req, err = http.NewRequest("GET", 
"/ws/v1/partition/default/usage/user/testuser", strings.NewReader(""))
+       vars := map[string]string{
+               "user":  "testuser",
+               "group": "testgroup",
+       }
+       req = mux.SetURLVars(req, vars)
+       assert.NilError(t, err, "Get User Resource Usage Handler request 
failed")
+       resp = &MockResponseWriter{}
+       var userResourceUsageDao *dao.UserResourceUsageDAOInfo
+       getUserResourceUsage(resp, req)
+       err = json.Unmarshal(resp.outputBytes, &userResourceUsageDao)
+       assert.NilError(t, err, "failed to unmarshal user resource usage dao 
response from response body: %s", string(resp.outputBytes))
+       assert.Equal(t, userResourceUsageDao.Queues.ResourceUsage.String(),
+               
resources.NewResourceFromMap(map[string]resources.Quantity{siCommon.CPU: 
1}).String())
+
+       // Test non-existing user query
+       req, err = http.NewRequest("GET", 
"/ws/v1/partition/default/usage/user/testNonExistingUser", 
strings.NewReader(""))
+       vars = map[string]string{
+               "user":  "testNonExistingUser",
+               "group": "testgroup",
+       }
+       req = mux.SetURLVars(req, vars)
+       assert.NilError(t, err, "Get User Resource Usage Handler request 
failed")
+       resp = &MockResponseWriter{}
+       getUserResourceUsage(resp, req)
+       assertUserExists(t, resp)
+

Review Comment:
   we should also do that for `users` and `groups` if we do it for `user` and 
`group`



##########
pkg/webservice/handlers.go:
##########
@@ -757,3 +780,22 @@ func getGroupsResourceUsage(w http.ResponseWriter, r 
*http.Request) {
                buildJSONErrorResponse(w, err.Error(), 
http.StatusInternalServerError)
        }
 }
+
+func getGroupResourceUsage(w http.ResponseWriter, r *http.Request) {
+       userManager := ugm.GetUserManager()

Review Comment:
   same as for the user: not until we have a group in the url that is not empty



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

Reply via email to