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/11/27 13:47:06 UTC
[GitHub] [incubator-yunikorn-core] manirajv06 opened a new pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
manirajv06 opened a new pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224
First cut implementation.
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (5dde515) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/683dea6673c75f24a3dfc892c31c12df212b0afe?el=desc) (683dea6) will **decrease** coverage by `0.07%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 62.99% -0.08%
==========================================
Files 59 59
Lines 4855 4883 +28
==========================================
+ Hits 3062 3076 +14
- Misses 1651 1662 +11
- Partials 142 145 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.63% <50.00%> (-0.46%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [683dea6...3930469](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (3930469) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/683dea6673c75f24a3dfc892c31c12df212b0afe?el=desc) (683dea6) will **decrease** coverage by `0.12%`.
> The diff coverage is `55.88%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 62.93% -0.13%
==========================================
Files 59 60 +1
Lines 4855 5232 +377
==========================================
+ Hits 3062 3293 +231
- Misses 1651 1773 +122
- Partials 142 166 +24
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `52.94% <55.88%> (-2.15%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `49.06% <0.00%> (-5.21%)` | :arrow_down: |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `61.58% <0.00%> (-3.69%)` | :arrow_down: |
| [pkg/scheduler/objects/node.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL25vZGUuZ28=) | `81.81% <0.00%> (-3.49%)` | :arrow_down: |
| [pkg/scheduler/partition.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb24uZ28=) | `62.79% <0.00%> (-2.11%)` | :arrow_down: |
| [pkg/common/resources/resources.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9yZXNvdXJjZXMvcmVzb3VyY2VzLmdv) | `96.02% <0.00%> (-1.48%)` | :arrow_down: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <0.00%> (-0.26%)` | :arrow_down: |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <0.00%> (ø)` | |
| [pkg/scheduler/health\_checker.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9oZWFsdGhfY2hlY2tlci5nbw==) | `83.52% <0.00%> (ø)` | |
| ... and [5 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [683dea6...48e87e6](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] yangwwei commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r636531149
##########
File path: pkg/webservice/handlers.go
##########
@@ -610,3 +612,50 @@ func getPartitions(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(), http.StatusInternalServerError)
}
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if !partitionExists {
+ buildJSONErrorResponse(w, "Partition is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ if !queueExists {
+ buildJSONErrorResponse(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ }
+ if len(vars) != 2 {
+ buildJSONErrorResponse(w, "Incorrect URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var appsDao []*dao.ApplicationDAOInfo
+ var partition = schedulerContext.GetPartition(vars["partition"])
Review comment:
What is the value of `vars["partition"]` here?
I think we should expect the partition name (without clusterID), e.g "default"
To get this to work, I think there needs some more work here. I think we need to add a function to retrieve the partition with the short form name. E.g `GetPartitionWithoutClusterID()`. Right now, this function `schedulerContext.GetPartition()` seems to require the full name form: `[rmID]default`.
could you please check this?
##########
File path: pkg/webservice/handlers_test.go
##########
@@ -836,3 +838,89 @@ func TestMetricsNotEmpty(t *testing.T) {
handler.ServeHTTP(rr, req)
assert.Assert(t, len(rr.Body.Bytes()) > 0, "Metrics response should not be empty")
}
+
+func TestGetQueueApplicationsHandler(t *testing.T) {
+ configs.MockSchedulerConfigByData([]byte(configDefault))
+ var err error
+ schedulerContext, err = scheduler.NewClusterContext(rmID, policyGroup)
+ assert.NilError(t, err, "Error when load clusterInfo from config")
+
+ assert.Equal(t, 1, len(schedulerContext.GetPartitionMapClone()))
+
+ // Check default partition
+ partitionName := "[" + rmID + "]default"
+ part := schedulerContext.GetPartition(partitionName)
+ assert.Equal(t, 0, len(part.GetApplications()))
+
+ // add a new app
+ app := newApplication("app-1", partitionName, "root.default", rmID)
+ err = part.AddApplication(app)
+ assert.NilError(t, err, "Failed to add Application to Partition.")
+ assert.Equal(t, app.CurrentState(), objects.New.String())
+ assert.Equal(t, 1, len(part.GetApplications()))
+
+ NewWebApp(schedulerContext, nil)
+
+ var req *http.Request
+ req, err = http.NewRequest("GET", "/ws/v1/partition/default/queue/root.default/applications", strings.NewReader(""))
+ vars := map[string]string{
+ "partition": "[rm-123]default",
+ "queue": "root.default",
+ }
Review comment:
Why the URL has partition: "default", but here in the vars we add "[rm-123]default" ?
There is some inconsistency here. Could you please clarify?
IMO, we should just use the partition name (without the clusterID) here, because clusterID is something internal.
##########
File path: pkg/webservice/handlers.go
##########
@@ -610,3 +612,50 @@ func getPartitions(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(), http.StatusInternalServerError)
}
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if !partitionExists {
+ buildJSONErrorResponse(w, "Partition is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ if !queueExists {
+ buildJSONErrorResponse(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ }
+ if len(vars) != 2 {
+ buildJSONErrorResponse(w, "Incorrect URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
Review comment:
why not using `buildJSONErrorResponse()` here?
##########
File path: pkg/webservice/handlers.go
##########
@@ -610,3 +612,50 @@ func getPartitions(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(), http.StatusInternalServerError)
}
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
Review comment:
NIT: can we directly assign the variables here:
```
partitionName, partitionExists := vars["partition"]
queueName, queueExists := vars["queue"]
```
##########
File path: pkg/webservice/handlers_test.go
##########
@@ -836,3 +838,89 @@ func TestMetricsNotEmpty(t *testing.T) {
handler.ServeHTTP(rr, req)
assert.Assert(t, len(rr.Body.Bytes()) > 0, "Metrics response should not be empty")
}
+
+func TestGetQueueApplicationsHandler(t *testing.T) {
+ configs.MockSchedulerConfigByData([]byte(configDefault))
+ var err error
+ schedulerContext, err = scheduler.NewClusterContext(rmID, policyGroup)
+ assert.NilError(t, err, "Error when load clusterInfo from config")
+
+ assert.Equal(t, 1, len(schedulerContext.GetPartitionMapClone()))
+
+ // Check default partition
+ partitionName := "[" + rmID + "]default"
Review comment:
we can leverage: `utils.GetNormalizedPartitionName`
--
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1affd7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **decrease** coverage by `0.41%`.
> The diff coverage is `61.76%`.
> :exclamation: Current head e1affd7 differs from pull request most recent head 4f44504. Consider uploading reports for the commit 4f44504 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.46% 63.05% -0.42%
==========================================
Files 60 60
Lines 5220 5232 +12
==========================================
- Hits 3313 3299 -14
- Misses 1747 1767 +20
- Partials 160 166 +6
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.79% <60.60%> (+0.66%)` | :arrow_up: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <100.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `16.00% <0.00%> (-4.00%)` | :arrow_down: |
| [pkg/scheduler/partition.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb24uZ28=) | `62.79% <0.00%> (-3.26%)` | :arrow_down: |
| [pkg/scheduler/objects/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `94.82% <0.00%> (-0.95%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `49.06% <0.00%> (+0.15%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb890bf...4f44504](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-core] manirajv06 commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-767429240
Taken care. Added more test cases.
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7764670) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.66%`.
> The diff coverage is `67.19%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
+ Coverage 63.46% 65.13% +1.66%
==========================================
Files 60 60
Lines 5220 5598 +378
==========================================
+ Hits 3313 3646 +333
- Misses 1747 1771 +24
- Partials 160 181 +21
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.78% <0.00%> (-0.49%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `63.33% <30.00%> (-10.58%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.18% <58.33%> (-0.27%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `56.58% <64.70%> (+7.67%)` | :arrow_up: |
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `58.88% <67.56%> (+4.74%)` | :arrow_up: |
| ... and [11 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb890bf...7764670](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (3930469) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/683dea6673c75f24a3dfc892c31c12df212b0afe?el=desc) (683dea6) will **decrease** coverage by `0.12%`.
> The diff coverage is `55.88%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 62.93% -0.13%
==========================================
Files 59 60 +1
Lines 4855 5232 +377
==========================================
+ Hits 3062 3293 +231
- Misses 1651 1773 +122
- Partials 142 166 +24
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `52.94% <55.88%> (-2.15%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `49.06% <0.00%> (-5.21%)` | :arrow_down: |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `61.58% <0.00%> (-3.69%)` | :arrow_down: |
| [pkg/scheduler/objects/node.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL25vZGUuZ28=) | `81.81% <0.00%> (-3.49%)` | :arrow_down: |
| [pkg/scheduler/partition.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb24uZ28=) | `62.79% <0.00%> (-2.11%)` | :arrow_down: |
| [pkg/common/resources/resources.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9yZXNvdXJjZXMvcmVzb3VyY2VzLmdv) | `96.02% <0.00%> (-1.48%)` | :arrow_down: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <0.00%> (-0.26%)` | :arrow_down: |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <0.00%> (ø)` | |
| [pkg/scheduler/health\_checker.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9oZWFsdGhfY2hlY2tlci5nbw==) | `83.52% <0.00%> (ø)` | |
| ... and [5 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [683dea6...3930469](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] yangwwei commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r562407705
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +522,49 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ if len(vars) != 2 {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ var queueName = vars["queue"]
+ if len(queueName) == 0 {
+ http.Error(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var partitionExists = false
+ var queueExists = false
+ var appsDao []*dao.ApplicationDAOInfo
+ lists := schedulerContext.GetPartitionMapClone()
+ for _, partition := range lists {
+ if partition.Name == vars["partition"] {
+ partitionExists = true
+ appList := partition.GetApplications()
Review comment:
This function is supposed to get all applications from a queue, why we need to loop the partition?
Can we leverage `queue#getCopyOfApps()`?
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +523,43 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if len(vars) != 2 || !partitionExists || !queueExists {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
Review comment:
It is better to explicitly what parameters are missing in the error message, that will be easier for the user to follow. Instead of asking them to look at the docs.
----------------------------------------------------------------
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-core] sunilgovind commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
sunilgovind commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r531666986
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +522,49 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ if len(vars) != 2 {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ var queueName = vars["queue"]
+ if len(queueName) == 0 {
+ http.Error(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var partitionExists = false
+ var queueExists = false
+ var appsDao []*dao.ApplicationDAOInfo
+ lists := schedulerContext.GetPartitionMapClone()
+ for _, partition := range lists {
+ if partition.Name == vars["partition"] {
+ partitionExists = true
+ appList := partition.GetApplications()
+ for _, app := range appList {
+ if strings.EqualFold(queueName, app.GetQueueName()) {
+ queueExists = true
+ appsDao = append(appsDao, getApplicationJSON(app))
+ }
+ }
+ }
+ }
+ if !partitionExists {
Review comment:
Do we need to loop to find the partition exists or not?
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +522,49 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ if len(vars) != 2 {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ var queueName = vars["queue"]
+ if len(queueName) == 0 {
+ http.Error(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var partitionExists = false
+ var queueExists = false
+ var appsDao []*dao.ApplicationDAOInfo
+ lists := schedulerContext.GetPartitionMapClone()
+ for _, partition := range lists {
+ if partition.Name == vars["partition"] {
+ partitionExists = true
+ appList := partition.GetApplications()
Review comment:
Is it safe to loop application 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-core] manirajv06 commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r636637435
##########
File path: pkg/webservice/handlers.go
##########
@@ -610,3 +612,50 @@ func getPartitions(w http.ResponseWriter, r *http.Request) {
buildJSONErrorResponse(w, err.Error(), http.StatusInternalServerError)
}
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if !partitionExists {
+ buildJSONErrorResponse(w, "Partition is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ if !queueExists {
+ buildJSONErrorResponse(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ }
+ if len(vars) != 2 {
+ buildJSONErrorResponse(w, "Incorrect URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
Review comment:
Taken care
##########
File path: pkg/webservice/handlers_test.go
##########
@@ -836,3 +838,89 @@ func TestMetricsNotEmpty(t *testing.T) {
handler.ServeHTTP(rr, req)
assert.Assert(t, len(rr.Body.Bytes()) > 0, "Metrics response should not be empty")
}
+
+func TestGetQueueApplicationsHandler(t *testing.T) {
+ configs.MockSchedulerConfigByData([]byte(configDefault))
+ var err error
+ schedulerContext, err = scheduler.NewClusterContext(rmID, policyGroup)
+ assert.NilError(t, err, "Error when load clusterInfo from config")
+
+ assert.Equal(t, 1, len(schedulerContext.GetPartitionMapClone()))
+
+ // Check default partition
+ partitionName := "[" + rmID + "]default"
+ part := schedulerContext.GetPartition(partitionName)
+ assert.Equal(t, 0, len(part.GetApplications()))
+
+ // add a new app
+ app := newApplication("app-1", partitionName, "root.default", rmID)
+ err = part.AddApplication(app)
+ assert.NilError(t, err, "Failed to add Application to Partition.")
+ assert.Equal(t, app.CurrentState(), objects.New.String())
+ assert.Equal(t, 1, len(part.GetApplications()))
+
+ NewWebApp(schedulerContext, nil)
+
+ var req *http.Request
+ req, err = http.NewRequest("GET", "/ws/v1/partition/default/queue/root.default/applications", strings.NewReader(""))
+ vars := map[string]string{
+ "partition": "[rm-123]default",
+ "queue": "root.default",
+ }
Review comment:
Will address in https://issues.apache.org/jira/browse/YUNIKORN-678
--
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-core] manirajv06 commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-766905523
> his function is supposed to get all applications from a queue, why we need to loop the partition?
> Can we leverage queue#getCopyOfApps()?
This has been already taken care in earlier commits. Can you check the recent changes?
----------------------------------------------------------------
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-core] codecov[bot] commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (5dde515) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/683dea6673c75f24a3dfc892c31c12df212b0afe?el=desc) (683dea6) will **decrease** coverage by `0.07%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 62.99% -0.08%
==========================================
Files 59 59
Lines 4855 4883 +28
==========================================
+ Hits 3062 3076 +14
- Misses 1651 1662 +11
- Partials 142 145 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.63% <50.00%> (-0.46%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [683dea6...5dde515](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] manirajv06 commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r563820902
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +523,43 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if len(vars) != 2 || !partitionExists || !queueExists {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
Review comment:
Makes sense. Taken care.
----------------------------------------------------------------
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-core] sunilgovind commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
sunilgovind commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r534126809
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +522,49 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ if len(vars) != 2 {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ var queueName = vars["queue"]
+ if len(queueName) == 0 {
+ http.Error(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var partitionExists = false
+ var queueExists = false
+ var appsDao []*dao.ApplicationDAOInfo
+ lists := schedulerContext.GetPartitionMapClone()
+ for _, partition := range lists {
+ if partition.Name == vars["partition"] {
+ partitionExists = true
+ appList := partition.GetApplications()
Review comment:
@wilfred-s cud u please share your thought on this comment?
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d44e581) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.79%`.
> The diff coverage is `65.32%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
+ Coverage 63.46% 65.26% +1.79%
==========================================
Files 60 60
Lines 5220 5677 +457
==========================================
+ Hits 3313 3705 +392
- Misses 1747 1784 +37
- Partials 160 188 +28
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.75% <0.00%> (-0.52%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `66.80% <22.58%> (-2.65%)` | :arrow_down: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `63.33% <30.00%> (-10.58%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `56.86% <65.64%> (+7.95%)` | :arrow_up: |
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `64.57% <66.44%> (+10.44%)` | :arrow_up: |
| ... and [12 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5cb9c4f...d44e581](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-core] manirajv06 commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-766905523
> his function is supposed to get all applications from a queue, why we need to loop the partition?
> Can we leverage queue#getCopyOfApps()?
This has been already taken care in earlier commits. Can you check the recent changes?
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7764670) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **increase** coverage by `1.66%`.
> The diff coverage is `67.19%`.
> :exclamation: Current head 7764670 differs from pull request most recent head d44e581. Consider uploading reports for the commit d44e581 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
+ Coverage 63.46% 65.13% +1.66%
==========================================
Files 60 60
Lines 5220 5598 +378
==========================================
+ Hits 3313 3646 +333
- Misses 1747 1771 +24
- Partials 160 181 +21
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <ø> (ø)` | |
| [pkg/scheduler/context.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9jb250ZXh0Lmdv) | `5.78% <0.00%> (-0.49%)` | :arrow_down: |
| [pkg/scheduler/nodes\_usage\_monitor.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9ub2Rlc191c2FnZV9tb25pdG9yLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/scheduler/objects/allocation.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FsbG9jYXRpb24uZ28=) | `100.00% <ø> (ø)` | |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `32.25% <ø> (+19.35%)` | :arrow_up: |
| [pkg/metrics/metrics\_collector.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL21ldHJpY3MvbWV0cmljc19jb2xsZWN0b3IuZ28=) | `63.33% <30.00%> (-10.58%)` | :arrow_down: |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `20.00% <50.00%> (ø)` | |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.18% <58.33%> (-0.27%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `56.58% <64.70%> (+7.67%)` | :arrow_up: |
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `58.88% <67.56%> (+4.74%)` | :arrow_up: |
| ... and [11 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5cb9c4f...d44e581](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
----------------------------------------------------------------
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-core] yangwwei commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
yangwwei commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-767311642
> > his function is supposed to get all applications from a queue, why we need to loop the partition?
> > Can we leverage queue#getCopyOfApps()?
>
> This has been already taken care in earlier commits. Can you check the recent changes?
Hi @manirajv06 I did read the latest change. Currently, it is like:
```
var queue = schedulerContext.GetQueue(queueName, vars["partition"])
if queue != nil {
appList := partition.GetApplications()
for _, app := range appList {
if strings.EqualFold(queueName, app.GetQueueName()) {
appsDao = append(appsDao, getApplicationJSON(app))
}
}
```
this loops all apps in the partition. I think we can change this to:
```
var queue = schedulerContext.GetQueue(queueName, vars["partition"])
if queue != nil {
appList := queue.GetCopyOfApps() // This needs to expose getCopyOfApps()
...
}
```
does. that make sense to you?
Second, looks like the code coverage has been dropped. Could you please add some negative tests, e.g wrong HTTP request to improve the UT coverage?
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (48e87e6) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/361d455c7f28b64fe6ae89b7a0dca310ace7783f?el=desc) (361d455) will **decrease** coverage by `0.01%`.
> The diff coverage is `64.70%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 63.05% -0.02%
==========================================
Files 60 60
Lines 5199 5232 +33
==========================================
+ Hits 3279 3299 +20
- Misses 1759 1767 +8
- Partials 161 166 +5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <ø> (ø)` | |
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.79% <64.70%> (+0.66%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [2bf2767...e1affd7](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] manirajv06 commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r563820902
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +523,43 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ _, partitionExists := vars["partition"]
+ _, queueExists := vars["queue"]
+ var queueName = vars["queue"]
+
+ if len(vars) != 2 || !partitionExists || !queueExists {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
Review comment:
Makes sense. Taken care.
----------------------------------------------------------------
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-core] yangwwei merged pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
yangwwei merged pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224
--
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (e1affd7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/2bf2767642274b47d94c022d4bc4f36f733da617?el=desc) (2bf2767) will **decrease** coverage by `0.01%`.
> The diff coverage is `61.76%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 63.05% -0.02%
==========================================
Files 60 60
Lines 5199 5232 +33
==========================================
+ Hits 3279 3299 +20
- Misses 1759 1767 +8
- Partials 161 166 +5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.79% <60.60%> (+0.66%)` | :arrow_up: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [2bf2767...e1affd7](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc) (48e87e6) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/683dea6673c75f24a3dfc892c31c12df212b0afe?el=desc) (683dea6) will **decrease** coverage by `0.01%`.
> The diff coverage is `64.70%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.06% 63.05% -0.02%
==========================================
Files 59 60 +1
Lines 4855 5232 +377
==========================================
+ Hits 3062 3299 +237
- Misses 1651 1767 +116
- Partials 142 166 +24
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <ø> (-0.26%)` | :arrow_down: |
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.79% <64.70%> (-0.29%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `49.06% <0.00%> (-5.21%)` | :arrow_down: |
| [pkg/metrics/scheduler.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3Mvc2NoZWR1bGVyLmdv) | `61.58% <0.00%> (-3.69%)` | :arrow_down: |
| [pkg/scheduler/objects/node.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL25vZGUuZ28=) | `81.81% <0.00%> (-3.49%)` | :arrow_down: |
| [pkg/scheduler/partition.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb24uZ28=) | `62.79% <0.00%> (-2.11%)` | :arrow_down: |
| [pkg/common/resources/resources.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9yZXNvdXJjZXMvcmVzb3VyY2VzLmdv) | `96.02% <0.00%> (-1.48%)` | :arrow_down: |
| [pkg/metrics/init.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree#diff-cGtnL21ldHJpY3MvaW5pdC5nbw==) | `62.96% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer). Last update [683dea6...48e87e6](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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-core] manirajv06 commented on a change in pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on a change in pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#discussion_r532726316
##########
File path: pkg/webservice/handlers.go
##########
@@ -521,3 +522,49 @@ func updateConfiguration(conf string) (string, error) {
}
return "", fmt.Errorf("config plugin not found")
}
+
+func getQueueApplications(w http.ResponseWriter, r *http.Request) {
+ vars := mux.Vars(r)
+ writeHeaders(w)
+ if len(vars) != 2 {
+ http.Error(w, "Mandatory parameters are missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ var queueName = vars["queue"]
+ if len(queueName) == 0 {
+ http.Error(w, "Queue is missing in URL path. Please check the usage documentation", http.StatusBadRequest)
+ return
+ }
+ queueErr := validateQueue(queueName)
+ if queueErr != nil {
+ http.Error(w, queueErr.Error(), http.StatusBadRequest)
+ return
+ }
+ var partitionExists = false
+ var queueExists = false
+ var appsDao []*dao.ApplicationDAOInfo
+ lists := schedulerContext.GetPartitionMapClone()
+ for _, partition := range lists {
+ if partition.Name == vars["partition"] {
+ partitionExists = true
+ appList := partition.GetApplications()
Review comment:
I think so as it is in handler side unlike variables in core objects/classes.
----------------------------------------------------------------
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-core] codecov[bot] edited a comment on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-735889296
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#224](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1affd7) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/5a1c19e280f66d6cb4fc206a675116260d6f2ff1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a1c19e) will **decrease** coverage by `0.41%`.
> The diff coverage is `61.76%`.
> :exclamation: Current head e1affd7 differs from pull request most recent head 7764670. Consider uploading reports for the commit 7764670 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
- Coverage 63.46% 63.05% -0.42%
==========================================
Files 60 60
Lines 5220 5232 +12
==========================================
- Hits 3313 3299 -14
- Misses 1747 1767 +20
- Partials 160 166 +6
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `54.79% <60.60%> (+0.66%)` | :arrow_up: |
| [pkg/scheduler/objects/queue.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL3F1ZXVlLmdv) | `69.44% <100.00%> (ø)` | |
| [pkg/scheduler/partition\_manager.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb25fbWFuYWdlci5nbw==) | `16.00% <0.00%> (-4.00%)` | :arrow_down: |
| [pkg/scheduler/partition.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9wYXJ0aXRpb24uZ28=) | `62.79% <0.00%> (-3.26%)` | :arrow_down: |
| [pkg/scheduler/objects/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `94.82% <0.00%> (-0.95%)` | :arrow_down: |
| [pkg/scheduler/objects/application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3NjaGVkdWxlci9vYmplY3RzL2FwcGxpY2F0aW9uLmdv) | `49.06% <0.00%> (+0.15%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cb890bf...7764670](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/224?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-core] manirajv06 commented on pull request #224: YUNIKORN-416: Add "applications" REST API to fetch list of applications
Posted by GitBox <gi...@apache.org>.
manirajv06 commented on pull request #224:
URL: https://github.com/apache/incubator-yunikorn-core/pull/224#issuecomment-833290583
@yangwwei @wilfred-s Made changes to use buildJSONErrorResponse() and added few more test cases. Can you please review?
--
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