[jira] [Updated] (YARN-5974) Remove direct reference to TimelineClientImpl

["Li Lu (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/YARN-5974?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Li Lu updated YARN-5974:
------------------------
    Attachment: YARN-5974-YARN-5355.001.patch

Took a look at all direct references to TimelineClientImpl in our code base. There are actually 3 types of non-trivial references:
a) Directly creating TimelineClientImpl in code. This is wrong. 
b) Creating anonymous class with a super class of TimelineClientImpl in test. 
c) Checking test-visible fields of TimelineClientImpl in related unit tests. 

The current (small) patch fixes all type a) problems in our code base. I believe type c) references are mostly fine since the author clearly knows the implication of the explicit test-visible method calls. I haven't decided yet on all type b) references. On one hand they're fine, since people also know the implication of an anonymous class in test code. On the other hand they're a little bit messy: once we'd like to split TimelineClientImpl we have to duplicate the work. We can have some intermediate class like TimelineClientImplV1ForTest extends TimelineClientImpl, and put that in test only. However, I'm not sure if the benefit justifies the efforts. Thoughts? 


> Remove direct reference to TimelineClientImpl
> ---------------------------------------------
>
>                 Key: YARN-5974
>                 URL: https://issues.apache.org/jira/browse/YARN-5974
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-5355
>            Reporter: Li Lu
>            Assignee: Li Lu
>              Labels: newbie++
>         Attachments: YARN-5974-YARN-5355.001.patch
>
>
> [~sjlee0]'s quick audit shows that things that are referencing TimelineClientImpl directly today:
> JobHistoryFileReplayMapperV1 (MR)
> SimpleEntityWriterV1 (MR)
> TestDistributedShell (DS)
> TestDSAppMaster (DS)
> TestNMTimelinePublisher (node manager)
> TestTimelineWebServicesWithSSL (AHS)
> This is not the right way to use TimelineClient and we should avoid direct reference to TimelineClientImpl as much as possible. 
> Any newcomers to the community are more than welcome to take this. If this remains unassigned for ~24hrs I'll jump in and do a quick fix. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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