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:
[email protected]
