X-czh commented on code in PR #20232:
URL: https://github.com/apache/flink/pull/20232#discussion_r1315734864


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/job/SubtaskExecutionAttemptDetailsInfo.java:
##########
@@ -203,7 +203,8 @@ public static SubtaskExecutionAttemptDetailsInfo create(
         final long now = System.currentTimeMillis();
 
         final TaskManagerLocation location = 
execution.getAssignedResourceLocation();
-        final String locationString = location == null ? "(unassigned)" : 
location.getHostname();
+        final String locationString =
+                location == null ? "(unassigned)" : 
location.getLocationString();

Review Comment:
   You made a good point. We should by no means rename the field directly as it 
breaks the backward-compatibility. I propose:
   1. Add a new field called "location" (already used in 
`JobExceptionsInfoWithHistory`) for storing the host + port info.
   2. Update the front-end to adopt the new location field.
   3. Rename the column name from "Host" to "Location" on the Web UI 
(previously we reached consensus on using "Host:port", but after a second 
thought, I think "Location" is more concise)
   4. Keep the old "host" field untouched. Notice that the info stored in the 
old "host" field is inconsistent, sometimes only host is stored, sometimes host 
+ port is stored, I'll leave them as they were with a few comments to keep 
being compatible.
   What do you think @1996fanrui @huwh ?



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

Reply via email to