1996fanrui commented on PR #21690: URL: https://github.com/apache/flink/pull/21690#issuecomment-1386900013
>> If it's ok, ReaderOutput#collect and RecordEmitter#emitRecord should return a boolean as well. And the loop will be moved from SourceOperator#emitNext to SourceReader#pollNext. > > I wouldn't go that far, at least not if strictly necessary. SourceReader is already part of the stable public API (@Public). Even if we could modify it without braking compatibility, it would make our future live more difficult, since we would have more complicated public API to support. And of course it would make the live of users implementing their sources more difficult as well. Sorry, I didn't realize that `ReaderOutput` and `SourceOutput` are also Public. The call chain of `NumberSequenceSource` is `SourceOperator#emitNext` -> `IteratorSourceReaderBase#pollNext` -> `SourceOutputWithWatermarks#collect` -> `DataOutput#emitRecord`. The `SourceOutputWithWatermarks` and `ReaderOutput` implement `SourceOutput`, if `DataOutput#emitRecord should return a boolean` and we add the loop in the `SourceOperator` or `SourceReaderBase`, we need add the return boolean for `SourceOutput#collect`. We will modify the `Public` api. It should be compatible for user due to these `collect` methods don't have the return value, so users didn't use it before. We just add the return value. However, I'm not sure if it's a good idea, if `SourceOutput#collect` wants to return other state in the future, what should we do? -- 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]
