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 2020/08/06 17:42:07 UTC

[GitHub] [incubator-yunikorn-site] manirajv06 opened a new pull request #14: [YUNIKORN-283] Support query params to filter the apps..

manirajv06 opened a new pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14


   ..using queue name/namespace for v1/apps endpoint


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

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



[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466831966



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Ah I see. You want to emphasize the parameter should be using the fully qualified name. How about:
   
   > The fully qualified queue name used to filter the applications that run within the given queue. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.




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

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



[GitHub] [incubator-yunikorn-site] manirajv06 commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466822754



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Can we say "Fully qualified queue name filters the applications based on exact match. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue. Passing only leaf queue name ("default") doesn't return the expected results." ?




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

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



[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466596999



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       > Passing only leaf queue name doesn't give the expected behaviour.
   
   Not completely sure what does this mean. Can you pls refine the text.

##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>

Review comment:
       is \> necessary? or just > is fine here?




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

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



[GitHub] [incubator-yunikorn-site] manirajv06 commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466822126



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>

Review comment:
       > is required to mark the close/end of param value. Backslash is required for escaping to avoid syntax error.




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

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



[GitHub] [incubator-yunikorn-site] yangwwei merged pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
yangwwei merged pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14


   


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

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



[GitHub] [incubator-yunikorn-site] manirajv06 commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466834451



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Looks good.




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

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



[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466831966



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Ah I see. You want to emphasize the parameter should be using the fully qualified name. How about:
   
   > The fully qualified queue name used to filter the applications that run within the given queue. For example, "/ws/v1/apps?queue=root.default" returns the applications running in "root.default" queue.




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

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



[GitHub] [incubator-yunikorn-site] yangwwei commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466831966



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Ah I see. You want to emphasize the parameter should be using the fully qualified name. How about:
   
   > The fully qualified queue name to filter the applications that run within the given queue. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.




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

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



[GitHub] [incubator-yunikorn-site] manirajv06 commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466834451



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>
+
+Fully qualified queue name filters the applications based on exact match. Passing only leaf queue name doesn't give the expected behaviour. For example, /ws/v1/apps?queue=root.default return the applications running in "root.default" queue.

Review comment:
       Looks good. Made changes. Please check




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

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



[GitHub] [incubator-yunikorn-site] manirajv06 commented on a change in pull request #14: [YUNIKORN-283] Support query params to filter the apps..

Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #14:
URL: https://github.com/apache/incubator-yunikorn-site/pull/14#discussion_r466822126



##########
File path: docs/api/scheduler.md
##########
@@ -85,6 +85,12 @@ Displays general information about the applications like used resources, queue n
 
 **Method** : `GET`
 
+**Query Params** : 
+
+1. queue=<fully qualified queue name\>

Review comment:
       \> is required to mark the close/end of param value. Backslash is required for escaping to avoid syntax error.




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

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