flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-8124) EventTimeTrigger (and other triggers) could have less specific generic types
Date Sun, 26 Nov 2017 16:55:00 GMT

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

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

GitHub user casidiablo opened a pull request:

    https://github.com/apache/flink/pull/5073

    [FLINK-8124] Make Trigger implementations more generic

    # What is the purpose of the change
    
    Flink provides implementations of the `Trigger<T, W extend Window>` class that are
unnecessarily
    specific. `EventTimeTrigger`, for instance, extends `Trigger<Object, TimeWindow>`.
    
    Since most window assigners provided also implement `WindowAssigner<Object, TimeWindow>`
this
    is usually not a problem. However, when implementing custom `WindowAssigner` it could
be problematic:
    
    ```java
    public class CustomWindowAssigner extends WindowAssigner<SomePojo, AnotherWindowImpl>
{
        ...
    
    	@Override
    	public Trigger<SomePojo, AnotherWindowImpl> getDefaultTrigger(StreamExecutionEnvironment
env) {
    		return EventTimeTrigger.create(); // compiler complains about typing
    	}
    
        ...
    }
    ```
    
    ## Brief change log
    
    This commit makes `Trigger` implementations generic, keeping them compatible with the
current
    windowing implementations as well as custom ones.
    
      - `EventTimeTrigger<T, W extends Window>`
      - `ProcessingTimeTrigger<T, W extends Window>`
      - `ContinuousEventTimeTrigger<T, W extends Window>`
      - `ContinuousProcessingTimeTrigger<T, W extends Window>`
      - `CountTrigger<T, W extends Window>`
    
    ## Verifying this change
    
    This change is already covered by existing tests, such as `WindowTranslationTest` or `DataStreamTest`.
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): no
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: yes
      - The serializers: no
      - The runtime per-record code paths (performance sensitive): no
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing,
Yarn/Mesos, ZooKeeper: no
      - The S3 file system connector: no
    
    ## Documentation
    
      - Does this pull request introduce a new feature? no


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/casidiablo/flink generic-triggers

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/5073.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5073
    
----
commit 12abf2d9b7c8d68991aad4e05683744bc3ed7a33
Author: Cristian <me@cristian.io>
Date:   2017-11-26T14:50:44Z

    [FLINK-8124] Make Trigger implementations more generic
    
    Flink provides implementations of the `Trigger<T, W extend Window>` class that are
unnecessarily
    specific. `EventTimeTrigger`, for instance, extends `Trigger<Object, TimeWindow>`.
    
    Since most window assigners provided also implement `WindowAssigner<Object, TimeWindow>`
this
    is usually not a problem. However, when implementing custom `WindowAssigner` it could
be problematic:
    
    ```java
    public class CustomWindowAssigner extends WindowAssigner<SomePojo, AnotherWindowImpl>
{
        ...
    
    	@Override
    	public Trigger<SomePojo, AnotherWindowImpl> getDefaultTrigger(StreamExecutionEnvironment
env) {
    		return EventTimeTrigger.create(); // compiler complains about typing
    	}
    
        ...
    }
    ```
    
    This commit makes `Trigger` implementations generic, keeping them compatible with the
current
    windowing implementations as well as custom ones.

----


> EventTimeTrigger (and other triggers) could have less specific generic types
> ----------------------------------------------------------------------------
>
>                 Key: FLINK-8124
>                 URL: https://issues.apache.org/jira/browse/FLINK-8124
>             Project: Flink
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Cristian
>            Priority: Minor
>
> When implementing custom WindowAssigners, it is possible to need different implementations
of the {{Window}} class (other than {{TimeWindow}}). In such cases, it is not possible to
use the existing triggers (e.g. {{EventTimeTrigger}}) because it extends from {{Trigger<Object,
TimeWindow>}} which is (unnecessarily?) specific.
> It should be possible to make that class more generic by using {{EventTimeTrigger<T,
W extends Window> extends Trigger<T, W>}} instead.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message