kl0u 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-533123881 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. 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).
---------------------------------------------------------------- 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] With regards, Apache Git Services
