[
https://issues.apache.org/jira/browse/SUBMARINE-60?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Szilard Nemeth updated SUBMARINE-60:
------------------------------------
Description:
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.
was:
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 (constructor is
preferred).
> 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: Szilard Nemeth
> Priority: Major
>
> 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)
