kl0u commented on pull request #13824:
URL: https://github.com/apache/flink/pull/13824#issuecomment-718710025


   > Given that now we have a transformation that is mapped to multiple 
operators, I would suggest to not reuse the `DataStreamSink` but create a new 
wrapper for the new `SinkTransformation` that will account for this difference.
   > 
   > In more detail, the new wrapper (I do not have a proposal for a name, for 
now let's call it `NewDataStreamSink`) could have different methods for setting 
the `uid` of the `writer`, the `committer` and the `globalCommitter` 
(`NewDataStreamSink withWriterUid(...)`, `NewDataStreamSink 
withCommiterUid(...)` and `NewDataStreamSink withGlobalCommitterUid(...)`), 
instead of trying to prepend or append things to the user-provided uid. I also 
propose to follow a builder pattern as you can see from the signatures above 
(the methods will return `this`), so that the user can write sth like:
   > 
   > ```
   > env.addSource()
   >       ...
   >       .addSink().withXXXUid(...)
   > ```
   > 
   > This will also allow us to have a validation step (if we want) that, for 
example, will check that if the `Sink` has a committer, then it has to also 
have a `committerUid`.
   > 
   > Then this `NewDataStreamSink` can forward all the details to the new 
`SinkTransformation` which will also have such methods, and from there, the 
translator will be able to set the `StreamNode` properties accordingly.
   > 
   > The same may make sense also for the `resource` settings but we can see 
later about that.
   > 
   > In general, I think that such a design that acknowledges that the new 
`Sink` does not fit in the old `Transformation` paradigm is more flexible and 
more future-proof.
   > 
   > What do you think @guoweiM and @aljoscha ?
   
   From an offline discussion with @guoweiM I think he is right to not go with 
my proposal above as it exposes more information about internal implementation 
than needed and this may tie our hands for the future.


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to