[
https://issues.apache.org/jira/browse/YARN-10148?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17040138#comment-17040138
]
Adam Antal commented on YARN-10148:
-----------------------------------
Thanks for the patch [~kmarton]!
As for the queue names, do they have some connection between them? I mean C and
C1 is not very descriptive in that sense (maybe foo/bar or A/B queues would be
better, but I'm also fine if you keep as it is).
Also I think it's dangerous to use Queue C and C1 in the new test cases, as
ACLsTestBase also has a "queueC":
{code:java}
protected static final String QUEUEC = "queueC";
{code}
and it just messes things up to name that the same way - e.g. in
{{TestCapacitySchedulerQueueACLs}} class:
{code:java}
@Override
public void updateConfigWithCAndC1Queues(String rootACL, String queueCAcl,
String queueC1Acl) throws IOException {
CapacitySchedulerConfiguration csConf =
(CapacitySchedulerConfiguration) getConf();
csConf.clear();
csConf.setQueues(CapacitySchedulerConfiguration.ROOT,
new String[] {QUEUE_C, QUEUEA, QUEUEB});
...
{code}
Here we mix the queues from different test cases.
Some indentation seems a bit strange to me, but locally running the checkstyle
plugin also did not report them. What I'm specifically thinking:
QueueACLsTestBase#L59-60:
{code:java}
abstract public void updateConfigWithCAndC1Queues(String rootACL,
String queueCAcl, String queueC1Acl) throws IOException;
{code}
QueueACLsTestBase#L137-138:
{code:java}
private void checkAccess(boolean rootAccess, boolean cAccess,
boolean c1Access) throws IOException {
{code}
and all along {{TestFairSchedulerQueueACLs#updateConfigWithCAndC1Queues}}.
Could you take a look at them?
I'd be more informative to have a failure message in
{{QueueACLTestBase#checkAccess}} 's assertEquals statements.
Why were the {{TestAbsoluteResourceConfiguration}} failure messages modified?
(I suppose they were auto-modified because the changes in
{{TestCapacitySchedulerQueueACLs}}). I think they were good, let's take it back.
I found GenericTestUtils#SYSPROP_TEST_DATA_DIR constant for "test.build.data",
you can also that in {{TestFairSchedulerQueueACLs}}.
> Add Unit test for queue ACL for both FS and CS
> ----------------------------------------------
>
> Key: YARN-10148
> URL: https://issues.apache.org/jira/browse/YARN-10148
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: scheduler
> Reporter: Kinga Marton
> Assignee: Kinga Marton
> Priority: Major
> Attachments: YARN-10148.001.patch, YARN-10148.002.patch
>
>
> Add some unit tests covering the queue ACL evaluation for both FS and CS.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
