xinbinhuang commented on code in PR #17873:
URL: https://github.com/apache/flink/pull/17873#discussion_r878322562
##########
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/JSONGenerator.java:
##########
@@ -184,6 +185,8 @@ private void decorateNode(Integer vertexID, ObjectNode
node) {
node.put(CONTENTS, vertex.getOperatorDescription());
+ node.put(SLOT_SHARING_GROUP, vertex.getSlotSharingGroup());
Review Comment:
Before adding the check, I want to think a little bit through this. I think
there're 3 scenarios w/ or w/o check
1. w/ check: if null, throw exception
2. w/ check: if null, avoid adding the field `slot_sharing_group`
3. w/o check: if null, return `"slot_sharing_group": null` in the JSON
I think I'm lean on 2 & 3. The reason to not throw an exception is that this
API locates around the "edge of of the call stacks" - it's printed out to the
console as an execution plan & DAG graph and it's likely not be depended by
other APIs directly. 3) has a small benefit to maintain a stable JSON schema -
it gives a clear message that the value has not been set properly (`null`),
while a missing field may requires someone to look into the code to find out
that the field is skipped because of `null`. WDYT @KarmaGYZ
--
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]
