[
https://issues.apache.org/jira/browse/YARN-9970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16983450#comment-16983450
]
Peter Bacsko commented on YARN-9970:
------------------------------------
1. Please write or generate a proper {{hashCode()}} method for {{QueueMapping}}:
{noformat}
@Override
public int hashCode() {
return super.hashCode();
}
{noformat}
2. Nit: add {{@Override}} annotation to {{QueueMapping.toString()}}
3. I have a problem with readability. For example, this is the original:
{noformat}
return new QueueMapping(mapping.getType(), mapping.getSource(),
queuePath.getLeafQueue(), queuePath.getParentQueue());
{noformat}
the new code:
{noformat}
return QueueMappingBuilder.create().type(mapping.getType())
.source(mapping.getSource()).queue(queuePath.getLeafQueue())
.parentQueue(queuePath.getParentQueue()).build();
{noformat}
To me it doesn't look significantly more readable. Maybe if we use one call per
line:
{noformat}
return QueueMappingBuilder.create()
.type(mapping.getType())
.source(mapping.getSource())
.queue(queuePath.getLeafQueue())
.parentQueue(queuePath.getParentQueue())
.build();
{noformat}
Nested builder invocations should be rewritten too:
{noformat}
verifyQueueMapping(QueueMappingTestDataBuilder.create()
.queueMapping(QueueMappingBuilder.create().type(MappingType.USER)
.source("a").queue("q1").build())
.inputUser("a").expectedQueue("q1").build());
{noformat}
My suggestion:
{noformat}
verifyQueueMapping(
QueueMappingTestDataBuilder.create()
.queueMapping(QueueMappingBuilder.create()
.type(MappingType.USER)
.source("a")
.queue("q1").build())
.inputUser("a")
.expectedQueue("q1").build()
);
{noformat}
> Refactor TestUserGroupMappingPlacementRule#verifyQueueMapping
> -------------------------------------------------------------
>
> Key: YARN-9970
> URL: https://issues.apache.org/jira/browse/YARN-9970
> Project: Hadoop YARN
> Issue Type: Bug
> Reporter: Manikandan R
> Assignee: Manikandan R
> Priority: Major
> Attachments: YARN-9970.001.patch, YARN-9970.002.patch,
> YARN-9970.003.patch
>
>
> Scope of this Jira is to refactor
> TestUserGroupMappingPlacementRule#verifyQueueMapping and QueueMapping class
> as discussed in
> https://issues.apache.org/jira/browse/YARN-9865?focusedCommentId=16971482&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16971482
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
