[GitHub] [flink] twalthr commented on pull request #18479: [FLINK-25387] Introduce ExecNodeMetadata

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

twalthr commented on pull request #18479:
URL: https://github.com/apache/flink/pull/18479#issuecomment-1023207736


   I kind of agree with @slinkydeveloper. The FLIP declares the full string for naming the operators `13_stream-exec-sink-1_upsert-materializer` and there you also see that I choose an underscore to separate `instance id and type id` and dash for the type id. It is fine to have all of those components in the `Context`, the context needs this to compute the `uid` for operators. However, one could argue that the instance id must not be part of the type id in the JSON plan. I could also imagine downstream tools would rather like to access a field then performing the splitting manually.
   
   However, I don't have a strong opinion here. If it is too complicated implementation wise to make `id` a separate JSON field as it was before, I'm also fine with the current solution.


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

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