[jira] [Commented] (SUBMARINE-60) Remove stubServiceClient from YarnServiceUtils

["Adam Antal (JIRA)" <ji...@apache.org>]

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

Adam Antal commented on SUBMARINE-60:
-------------------------------------

Thanks for assigning this to me [~snemeth]. Let's retrigger jenkins for the latest patch.

> Remove stubServiceClient from YarnServiceUtils
> ----------------------------------------------
>
>                 Key: SUBMARINE-60
>                 URL: https://issues.apache.org/jira/browse/SUBMARINE-60
>             Project: Hadoop Submarine
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Adam Antal
>            Priority: Major
>         Attachments: SUBMARINE-60.001.patch, SUBMARINE-60.002.patch, SUBMARINE-60.002.patch, SUBMARINE-60.003.patch, SUBMARINE-60.003.patch
>
>
> We have a field in YarnServiceUtils: org.apache.hadoop.yarn.submarine.runtimes.yarnservice.YarnServiceUtils#stubServiceClient
> There's a setter for this field, marked with VisibleForTesting and the test code uses this setter to set the value of this field to a mock.
> Then, when createServiceClient gets called from the production code, it first checks if we have this field set. If so, we return it, otherwise we create a normal app admin client.
> This is an anti-pattern to just have test-related fields or methods in the production code.
>  
> Currently, YarnServiceJobSubmitter and YarnServiceJobMonitor are the only users of org.apache.hadoop.yarn.submarine.runtimes.yarnservice.YarnServiceUtils#createServiceClient.
> This static could be easily replaced with a factory that receives the current Configuration object then returns the AppAdminClient. The test should inject the mock factory either via the constructor or with a setter.



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