[GitHub] [hadoop] brumi1024 commented on a diff in pull request #4165: YARN-10850. TimelineService v2 lists containers for all attempts when…

[GitBox <gi...@apache.org>]

brumi1024 commented on code in PR #4165:
URL: https://github.com/apache/hadoop/pull/4165#discussion_r870230374


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AHSv2ClientImpl.java:
##########
@@ -137,9 +137,8 @@ public List<ContainerReport> getContainers(ApplicationAttemptId
     ApplicationId appId = applicationAttemptId.getApplicationId();
     ApplicationReport appReport = getApplicationReport(appId);
     Map<String, String> filters = new HashMap<>();
-    filters.put("infofilters", "SYSTEM_INFO_PARENT_ENTITY eq {\"id\":\"" +
-        applicationAttemptId.toString() +
-        "\",\"type\":\"YARN_APPLICATION_ATTEMPT\"}");
+    filters.put("infofilters", "SYSTEM_INFO_PARENT_ENTITY eq "
+        + "{\"type\":\"YARN_APPLICATION_ATTEMPT\",\"id\":\"" + applicationAttemptId + "\"}");

Review Comment:
   It is, and it was before the change, as this basically puts together a GET query, where it's expected to get the applicationAttempt string form. Also it's not likely that the ApplicationAttemptId will be changed because it's marked public and stable, and changing this means that the applicationAttempt format will change everywhere starting from the cli tools all the way to logging.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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