[GitHub] [yunikorn-site] wilfred-s commented on a diff in pull request #188: [YUNIKORN-1315] Document how to use MaxApplication enforcement feature

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

wilfred-s commented on code in PR #188:
URL: https://github.com/apache/yunikorn-site/pull/188#discussion_r990447006


##########
docs/user_guide/queue_config.md:
##########
@@ -262,9 +272,10 @@ The star "*" is the wildcard character and matches all users or groups.
 Duplicate entries in the lists are ignored and do not cause a parsing error.
 Specifying a star beside other list elements is not allowed.
 
-_maxapplications_ is an unsigned integer value, larger than 1, which allows you to limit the number of running applications for the configured user or group.
+_maxapplications_ is an integer value, larger than 1, which allows you to limit the number of running applications for the configured user or group.
 Specifying a zero maximum applications limit is not allowed as it would implicitly deny access.
-Denying access must be handled via the ACL entries.  
+Denying access must be handled via the ACL entries.
+_parent_ queue _maxapplications_ value must be larger than _parent_ queue _maxapplications_ value.

Review Comment:
   This is not correct: the maxapplications for the user and or group must be smaller than the value set for the queue. However we do not enforce that at the moment. If we add a validation it will not be against the parent queue.



##########
docs/user_guide/queue_config.md:
##########
@@ -262,9 +272,10 @@ The star "*" is the wildcard character and matches all users or groups.
 Duplicate entries in the lists are ignored and do not cause a parsing error.
 Specifying a star beside other list elements is not allowed.
 
-_maxapplications_ is an unsigned integer value, larger than 1, which allows you to limit the number of running applications for the configured user or group.
+_maxapplications_ is an integer value, larger than 1, which allows you to limit the number of running applications for the configured user or group.

Review Comment:
   unsigned was added on purpose as the parameter is defined as an `uint64` in the code. If you set a -1 an error unmarshalling the json will occur while a 0 should return a validation error. Probably better to keep the unsigned in the doc.



##########
docs/user_guide/queue_config.md:
##########
@@ -133,6 +133,7 @@ Supported parameters for the queues:
 * name
 * parent
 * queues
+* maxApplications

Review Comment:
   should be all lowercase like the example



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