yxu-valleytider commented on issue #9581: [FLINK-13864][streaming]: Modify the StreamingFileSink Builder interface to allow for easier subclassing of StreamingFileSink URL: https://github.com/apache/flink/pull/9581#issuecomment-533209039 > Hi @yxu-valleytider , I still have the following concerns that make me feel uncomfortable about merging this PR: > > 1. The user should know that he has to write his own `forRowFormat` / `forBulkFormat` in order for them to return the correct builder, and this can only be "implicit" knowledge. > 2. The user should override the `build` method of the builder he provides. > 3. It may break user-code, if the user was explicitly stating in his program the return type of the `forRowFormat` or `forBulkFormat`. This is because we add a new type parameter. @kl0u I can see 3) is more or less a concern. This PR does introduce quite a bit of changes to the interface of the StreamingFileSink -- the original hope is there are not so many users of `StreamingFileSink` yet hence impact is limited. > Also some other issues that can be tackled easily though are: > > 1. The constructor should not take a `BucketsBuilder` but we have to have 2 constructors, one for row and one for bulk. If not, then the user may create an invalid `StreamingFileSink`. > 2. The user cannot change the type of the `BucketID` (although this may not be that important). @aljoscha In terms of a step back, it would something extending [this branch](https://github.com/apache/flink/compare/master...kailashhd:FLINK-12539). If you are OK with adding a bunch of getter functions to retrieve the `private` member fields of the class.
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services
