Mit Desai commented on YARN-2890:

I'm sorry for taking so long to get back [~hitesh].
bq. In the future, it would be good if your patches are versioned to avoid 
I will keep that in mind. But once a newer patch is submitted with the same 
name name, the older one gets grayed out. It was convenient for me. But if that 
creates confusion, I don't mind numbering the patches.
bq. * testTimelineServiceStartInMiniCluster() - is there a reason why a job is 
run when timeline is enabled but not run when it is disabled?
nice catch. That was by mistake. We do not need to test job run in this test.
bq. * should be a job run be needed here in the first place given the name of 
the test?
nope. we don't need a job run. I will update the patch to remove the that
bq. * might be better to move the testing of job runs based on absence/presence 
of timeline to a separate test
That can be done. But I don't think we are testing job runs here. Just the 
check for timeline service should be enough.
bq. * testMRTimelineEventHandling, testMapreduceJobTimelineServiceEnabled, 
testMapreduceJobTimelineServiceEnabled: is there a need to change all of them?
These tests are changed because checking the config value seems to be a better 
way to turn on/off the timelineserver than just passing a boolean value
bq. * there does not seem to be a code path that tests timeline being enabled 
by passing the enableAHS value in the ctor if all these are changed.
I would like to keep these tests as they are and add a new test that tests that 
code path. What do you think about that?

[~hitesh], I will wait for your feedback before I post another patch for adding 
the test to test the enableAHS flag and remove the job run from the test that I 

> MiniMRYarnCluster should turn on timeline service if configured to do so
> ------------------------------------------------------------------------
>                 Key: YARN-2890
>                 URL: https://issues.apache.org/jira/browse/YARN-2890
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 2.6.0
>            Reporter: Mit Desai
>            Assignee: Mit Desai
>         Attachments: YARN-2890.patch, YARN-2890.patch, YARN-2890.patch, 
> YARN-2890.patch, YARN-2890.patch
> Currently the MiniMRYarnCluster does not consider the configuration value for 
> enabling timeline service before starting. The MiniYarnCluster should only 
> start the timeline service if it is configured to do so.

This message was sent by Atlassian JIRA

Reply via email to