hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Szilard Nemeth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-9214) Add AbstractYarnScheduler#getValidQueues method to resolve duplicate code
Date Thu, 28 Mar 2019 19:50:00 GMT

    [ https://issues.apache.org/jira/browse/YARN-9214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16804244#comment-16804244
] 

Szilard Nemeth commented on YARN-9214:
--------------------------------------

Hi [~jiwq]!

This is a nice refactor, in overall!

Here are my comments: 
 # Imports changes are confusing, I guess you Organized imports with your IDE. Could you
please revert those changes and only add the new imports to the bottom?
 # The name of the extracted method "getValidQueues" is misleading: The method is not about
getting valid queues, it returns apps belong to the specified queue. I would rename it to
getAppsFromQueue() or something like that. 
 # I know you just extracted the method, but I think it's okay to modify the error message
a bit to: "The specified queue: " + queueName + " does not exist!" (queue with lowercase Q
in the beginning, does not instead of doesn't)

Apart from these, I'm okay with the patch!

Thanks!

> Add AbstractYarnScheduler#getValidQueues method to resolve duplicate code 
> --------------------------------------------------------------------------
>
>                 Key: YARN-9214
>                 URL: https://issues.apache.org/jira/browse/YARN-9214
>             Project: Hadoop YARN
>          Issue Type: Improvement
>    Affects Versions: 3.1.0, 3.2.0, 2.9.2, 3.0.3, 2.8.5
>            Reporter: Wanqiang Ji
>            Assignee: Wanqiang Ji
>            Priority: Major
>             Fix For: 3.3.0
>
>         Attachments: YARN-9214.001.patch, YARN-9214.002.patch
>
>
> *AbstractYarnScheduler#moveAllApps* and *AbstractYarnScheduler#killAllAppsInQueue* had
the same code segment. So I think we need a method to handle it named *AbstractYarnScheduler#getValidQueues*.
Apart from this we need add the doc comment to expound why exists.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org


Mime
View raw message