[GitHub] [flink] StephanEwen edited a comment on pull request #13784: [FLINK-19689][connectors/common] API improvements to the Sources.

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

StephanEwen edited a comment on pull request #13784:
URL: https://github.com/apache/flink/pull/13784#issuecomment-716619665


   @becketqin Thanks, I somehow overlooked your question about adding the checkpointId to `snapshotState()`.
   
   We have the following two options:
     - Adding the `checkpointId` to `snapshotState()` and adding the `onCheckpointComplete(long checkpointId)` method.
     - Adding a new interface `CheckpointListener` or so that the readers / enumerators can implement. That interface would have the `onCheckpoint(checkpointId)` and `onCheckpointComplete(checkpointId)` methods.
   
   The first option would have the advantage of fewer interfaces / methods.
   
   The second option would have the advantage that we could keep the checkpoint related stuff out of the core interface (as was one of the original design goals of FLIP-27, if I recall correctly) and make it optional for implementations to observe checkpoints.
   That would be a bit similar to how a transformation function implements an additional interface if it wants callbacks on checkpoints and their completion.
   One more advantage here would be that it is actually non-breaking, depending on how much we care about that at this still early stage of the API.
   
   I am not very opinionated, maybe leaning just slightly towards the second option. But I would also be fine with adding this all to the core interface, if you strongly prefer that option.


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