[ 
https://issues.apache.org/jira/browse/FLINK-3647?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15342069#comment-15342069
 ] 

ASF GitHub Bot commented on FLINK-3647:
---------------------------------------

Github user aljoscha commented on the issue:

    https://github.com/apache/flink/pull/2124
  
    This looks very good! I just have a few minor comments:
    
    * It's very good that you fixed typos and added comments/Javadoc to 
previously undocumented code. I would like to see them as a separate PR, 
though, and keep this PR only to the feature mentioned in the Jiras. 
    
    * You added `TestTimeServiceProvider` in two tests. I think it makes sense 
to move this to the same package as `DefaultTimerService` and clearly mark it 
as being for testing only. On a related note, it might make sense to normalize 
the names, right now there is `*TimeServiceProvider` and `*TimerService`.
    
    * You added the additional method `registerTimer(final long timestamp, 
final Runnable target)`. I imagine it is because the sources register their 
watermark timers as a `Runnable`. I think it would be better to keep having 
only `Triggerable` here. The watermark emitters should also work as a 
`Triggerable`.



> Change StreamSource to use Processing-Time Clock Service
> --------------------------------------------------------
>
>                 Key: FLINK-3647
>                 URL: https://issues.apache.org/jira/browse/FLINK-3647
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Streaming
>            Reporter: Aljoscha Krettek
>            Assignee: Kostas Kloudas
>
> Currently, the {{StreamSource.AutomaticWatermarkContext}} has it's own timer 
> service. This should be changed to use the Clock service introduced in 
> FLINK-3646 to make watermark emission testable by providing a custom Clock.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to