xinyuiscool commented on PR #22576: URL: https://github.com/apache/beam/pull/22576#issuecomment-1218377898
I am not a big fan of reusing the transform name to carry info of the transform, and the changes to the Op/OpAdapter seems quite confusing (the name of fullName is not exactly defined anywhere) and easy to miss (we need to add the fullName everywhere). In general, it's not a very good solution to me to show user information about the transform in this approach. The better way, I think, is to use the DisplayData in PTransform to carry custom info about the transform. You can check out an example in AvroSourceTests.testDisplayData. You can also take a look at the other impls of HasDisplayData. So basically: 1) Users will populate the customized DisplayData in their ptransforms. The DisplayData can be populated to each sub-transform by traversing the dag if needed in the user code. 2) Now the transform has the info in it's DisplayData. During translation, we can set the DisplayData for the current transform when we traverse the pipeline. I wouldn't mind adding a static method in the translation context for getCurrentTransformDisplayData(), to avoid adding the DisplayData everywhere. 3) Now we have the DisplayData available when creating the OpAdapter, it can passed into the constructor as a member var there since it's serializable. Then we can use that to log / invoke the observer. -- 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]
