Sanil15 commented on a change in pull request #1421:
URL: https://github.com/apache/samza/pull/1421#discussion_r477768099
##########
File path:
samza-core/src/test/java/org/apache/samza/clustermanager/TestContainerPlacementActions.java
##########
@@ -672,18 +667,23 @@ public void
testAlwaysMoveToAnyHostForHostAffinityDisabled() throws Exception {
Map<String, String> conf = new HashMap<>();
conf.putAll(getConfigWithHostAffinityAndRetries(false, 1, true));
SamzaApplicationState state =
- new
SamzaApplicationState(getJobModelManagerWithHostAffinity(ImmutableMap.of("0",
"host-1", "1", "host-2")));
+ new
SamzaApplicationState(JobModelManagerTestUtil.getJobModelManager(getConfig(),
2, this.server));
ClusterResourceManager.Callback callback =
mock(ClusterResourceManager.Callback.class);
MockClusterResourceManager clusterResourceManager = new
MockClusterResourceManager(callback, state);
+ LocalityManager mockLocalityManager = mock(LocalityManager.class);
Review comment:
redundant, isn't this already done in setup?
##########
File path:
samza-core/src/test/java/org/apache/samza/clustermanager/TestContainerPlacementActions.java
##########
@@ -558,14 +548,19 @@ public Void answer(InvocationOnMock invocation) {
public void testContainerPlacementsForJobRunningInDegradedState() throws
Exception {
// Set failure after retries to false to enable job running in degraded
state
config = new MapConfig(configVals,
getConfigWithHostAffinityAndRetries(true, 1, false));
- state = new
SamzaApplicationState(getJobModelManagerWithHostAffinity(ImmutableMap.of("0",
"host-1", "1", "host-2")));
+ state = new
SamzaApplicationState(JobModelManagerTestUtil.getJobModelManager(getConfig(),
2, this.server));
callback = mock(ClusterResourceManager.Callback.class);
MockClusterResourceManager clusterResourceManager = new
MockClusterResourceManager(callback, state);
ClusterManagerConfig clusterManagerConfig = new
ClusterManagerConfig(config);
- containerManager = spy(new
ContainerManager(containerPlacementMetadataStore, state,
clusterResourceManager, true, false));
+ LocalityManager mockLocalityManager = mock(LocalityManager.class);
Review comment:
redundant, isn't this already done in setup?
##########
File path:
samza-core/src/main/scala/org/apache/samza/coordinator/JobModelManager.scala
##########
@@ -167,15 +167,18 @@ object JobModelManager extends Logging {
*/
def getProcessorLocality(config: Config, localityManager: LocalityManager) =
{
Review comment:
Why do we need this here can't the callee of this not call
LocalityManager since we are already making them independent
##########
File path:
samza-core/src/test/java/org/apache/samza/clustermanager/TestContainerPlacementActions.java
##########
@@ -807,16 +807,21 @@ public Void answer(InvocationOnMock invocation) {
@Test(expected = NullPointerException.class)
public void testBadControlRequestRejected() throws Exception {
SamzaApplicationState state =
- new
SamzaApplicationState(getJobModelManagerWithHostAffinity(ImmutableMap.of("0",
"host-1", "1", "host-2")));
+ new
SamzaApplicationState(JobModelManagerTestUtil.getJobModelManager(getConfig(),
2, this.server));
ClusterResourceManager.Callback callback =
mock(ClusterResourceManager.Callback.class);
MockClusterResourceManager clusterResourceManager = new
MockClusterResourceManager(callback, state);
+ LocalityManager mockLocalityManager = mock(LocalityManager.class);
Review comment:
redundant, isn't this already done in setup?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
