brumi1024 commented on a change in pull request #3763:
URL: https://github.com/apache/hadoop/pull/3763#discussion_r786070977



##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +112,76 @@ public void testGetAppAttemptReport() throws IOException, 
YarnException {
         isEqualTo("test original tracking url");
   }
 
+  @Test
+  public void testGetContainerByAppAttempt() throws IOException, YarnException 
{
+    int applicationId = 1;
+    ApplicationId appId = ApplicationId.newInstance(0, applicationId);
+
+    ApplicationAttemptId appAttemptId =
+            ApplicationAttemptId.newInstance(appId, 1);
+
+    ContainerId containerId = ContainerId.newContainerId(appAttemptId, 1);
+
+    when(spyTimelineReaderClient.getContainerEntities(
+            appId, "ALL",
+            ImmutableMap.<String, String>builder()
+                    .put("appattemptId", appAttemptId.toString())
+                    .build(),

Review comment:
       Sorry, I missed this last round: ImmutableMap.of(K key, V value) is way 
more concise.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +112,76 @@ public void testGetAppAttemptReport() throws IOException, 
YarnException {
         isEqualTo("test original tracking url");
   }
 
+  @Test
+  public void testGetContainerByAppAttempt() throws IOException, YarnException 
{
+    int applicationId = 1;
+    ApplicationId appId = ApplicationId.newInstance(0, applicationId);
+
+    ApplicationAttemptId appAttemptId =
+            ApplicationAttemptId.newInstance(appId, 1);
+
+    ContainerId containerId = ContainerId.newContainerId(appAttemptId, 1);
+
+    when(spyTimelineReaderClient.getContainerEntities(
+            appId, "ALL",
+            ImmutableMap.<String, String>builder()
+                    .put("appattemptId", appAttemptId.toString())
+                    .build(),
+            0, null))
+            .thenReturn(Arrays.asList(createContainerEntity(containerId)));
+
+    when(spyTimelineReaderClient.getApplicationEntity(appId, "ALL", null))
+            .thenReturn(createApplicationTimelineEntity(appId, true,
+                    false));
+
+    List<ContainerReport> containerList = client.getContainers(appAttemptId);
+
+    assertThat(containerList.size()).isEqualTo(1);
+
+    assertThat(containerList.get(0).getContainerId().getApplicationAttemptId()
+            .getApplicationId().getId()).isEqualTo(applicationId);
+  }
+
+  @Test
+  public void testGetMultipleContainersByAppAttempt() throws IOException, 
YarnException {
+    int multipleContainerSize = 4;
+    int appMultipleId = 3;
+    int appAttemptMultipleId = 3;
+    ApplicationId appMultiple = ApplicationId.newInstance(0, appMultipleId);
+    ApplicationAttemptId appAttemptIdMultiple =
+            ApplicationAttemptId.newInstance(appMultiple, 
appAttemptMultipleId);
+
+    List<TimelineEntity> containerEntities = new ArrayList<>();
+    for (int newEntityIdx = 0; newEntityIdx < multipleContainerSize; 
++newEntityIdx) {
+      containerEntities.add(createContainerEntity(ContainerId.newContainerId(
+              appAttemptIdMultiple, newEntityIdx)));
+    }
+
+    when(spyTimelineReaderClient.getContainerEntities(
+            appMultiple, "ALL",
+            ImmutableMap.<String, String>builder()
+                    .put("appattemptId", appAttemptIdMultiple.toString())
+                    .build(),

Review comment:
       Same as above: ImmutableList.of.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +112,76 @@ public void testGetAppAttemptReport() throws IOException, 
YarnException {
         isEqualTo("test original tracking url");
   }
 
+  @Test
+  public void testGetContainerByAppAttempt() throws IOException, YarnException 
{
+    int applicationId = 1;
+    ApplicationId appId = ApplicationId.newInstance(0, applicationId);
+
+    ApplicationAttemptId appAttemptId =
+            ApplicationAttemptId.newInstance(appId, 1);
+
+    ContainerId containerId = ContainerId.newContainerId(appAttemptId, 1);
+
+    when(spyTimelineReaderClient.getContainerEntities(
+            appId, "ALL",
+            ImmutableMap.<String, String>builder()
+                    .put("appattemptId", appAttemptId.toString())
+                    .build(),
+            0, null))
+            .thenReturn(Arrays.asList(createContainerEntity(containerId)));
+
+    when(spyTimelineReaderClient.getApplicationEntity(appId, "ALL", null))
+            .thenReturn(createApplicationTimelineEntity(appId, true,
+                    false));
+
+    List<ContainerReport> containerList = client.getContainers(appAttemptId);
+
+    assertThat(containerList.size()).isEqualTo(1);
+
+    assertThat(containerList.get(0).getContainerId().getApplicationAttemptId()
+            .getApplicationId().getId()).isEqualTo(applicationId);
+  }
+
+  @Test
+  public void testGetMultipleContainersByAppAttempt() throws IOException, 
YarnException {
+    int multipleContainerSize = 4;
+    int appMultipleId = 3;
+    int appAttemptMultipleId = 3;

Review comment:
       For easier readability I think sticking with the previous naming 
structure would be beneficial: applicationId, applicationAttemptId, 
numContainers. And the objects could have a postfix like appAttempIdObject or 
something like that.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -32,6 +33,7 @@
 import org.apache.hadoop.yarn.api.records.ContainerState;
 import org.apache.hadoop.yarn.api.records.Priority;
 import org.apache.hadoop.yarn.api.records.YarnApplicationState;
+import org.apache.hadoop.yarn.api.records.timelineservice.ContainerEntity;

Review comment:
       Nit: this is unused.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAHSv2ClientImpl.java
##########
@@ -107,6 +112,76 @@ public void testGetAppAttemptReport() throws IOException, 
YarnException {
         isEqualTo("test original tracking url");
   }
 
+  @Test
+  public void testGetContainerByAppAttempt() throws IOException, YarnException 
{
+    int applicationId = 1;
+    ApplicationId appId = ApplicationId.newInstance(0, applicationId);
+
+    ApplicationAttemptId appAttemptId =
+            ApplicationAttemptId.newInstance(appId, 1);
+
+    ContainerId containerId = ContainerId.newContainerId(appAttemptId, 1);
+
+    when(spyTimelineReaderClient.getContainerEntities(
+            appId, "ALL",
+            ImmutableMap.<String, String>builder()
+                    .put("appattemptId", appAttemptId.toString())
+                    .build(),
+            0, null))
+            .thenReturn(Arrays.asList(createContainerEntity(containerId)));
+
+    when(spyTimelineReaderClient.getApplicationEntity(appId, "ALL", null))
+            .thenReturn(createApplicationTimelineEntity(appId, true,
+                    false));
+
+    List<ContainerReport> containerList = client.getContainers(appAttemptId);
+
+    assertThat(containerList.size()).isEqualTo(1);
+
+    assertThat(containerList.get(0).getContainerId().getApplicationAttemptId()
+            .getApplicationId().getId()).isEqualTo(applicationId);
+  }
+
+  @Test
+  public void testGetMultipleContainersByAppAttempt() throws IOException, 
YarnException {
+    int multipleContainerSize = 4;
+    int appMultipleId = 3;
+    int appAttemptMultipleId = 3;
+    ApplicationId appMultiple = ApplicationId.newInstance(0, appMultipleId);
+    ApplicationAttemptId appAttemptIdMultiple =
+            ApplicationAttemptId.newInstance(appMultiple, 
appAttemptMultipleId);
+
+    List<TimelineEntity> containerEntities = new ArrayList<>();
+    for (int newEntityIdx = 0; newEntityIdx < multipleContainerSize; 
++newEntityIdx) {

Review comment:
       newEntityIdx -> this is essentially a containerId, can you please rename 
it?




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to