[GitHub] [flink] kl0u commented on pull request #13824: [FLINK-19736] Add the SinkTransformation

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

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:
users@infra.apache.org