You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by aljoscha <gi...@git.apache.org> on 2017/02/08 10:26:38 UTC

[GitHub] flink pull request #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFu...

GitHub user aljoscha opened a pull request:

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

    [FLINK-4997] [streaming] Introduce ProcessWindowFunction

    This is an updated/enhanced version of #2756.
    
    I did roughly these changes:
     - Add support for the `AggregatingFunction`/`ProcessWindowFunction` combination
     - Mark the new methods/interfaces as `@PublicEvolving`
     - Change how the Scala wrapper functions work because we recently moved all of them to use `WrappingFunction`. This is not complete yet and I created issues for fixing that: https://issues.apache.org/jira/browse/FLINK-5740 and https://issues.apache.org/jira/browse/FLINK-5741
     - Add tests in `WindowTranslationTest.java` and `WindowTranslationTest.scala`.
    
    @VenturaDelMonte and @manuzhang, would you like to have a look at this. I took quite a while because there was some upheaval in the internals of that part of the code and the tests. I'm sorry for that.

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

    $ git pull https://github.com/aljoscha/flink process-window-function

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

    https://github.com/apache/flink/pull/3285.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 #3285
    
----
commit 6f476f92f22b661f26d322615af6a52eca01c69c
Author: Aljoscha Krettek <al...@gmail.com>
Date:   2017-02-07T09:54:54Z

    [hotfix] Fix trailing whitespace in WindowedStream.java

commit ab74f4142ddc4aab6e76563b32464d526f719ae7
Author: Ventura Del Monte <ve...@gmail.com>
Date:   2016-11-23T17:00:23Z

    [FLINK-4997] [streaming] Introduce ProcessWindowFunction

commit b6759f446bad5170fabc803103c764f204412301
Author: Ventura Del Monte <ve...@gmail.com>
Date:   2016-11-09T09:49:47Z

    [FLINK-4997] [streaming] Add ProcessWindowFunction to Scala API

commit a9791e0e7526bfd419e72ac749ce068f18df857a
Author: Aljoscha Krettek <al...@gmail.com>
Date:   2017-02-07T13:38:25Z

    [FLINK-4997] Add ProcessWindowFunction support for .aggregate()

commit 01048feccd5aced42b1979a3bf7b355443c08f5b
Author: Aljoscha Krettek <al...@gmail.com>
Date:   2016-11-24T07:14:48Z

    [FLINK-5237] Consolidate and harmonize Window Translation Tests

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFu...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFunction

Posted by manuzhang <gi...@git.apache.org>.
Github user manuzhang commented on the issue:

    https://github.com/apache/flink/pull/3285
  
    LGTM. Really excited to see we move further in this direction, and faster if possible. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFunction

Posted by VenturaDelMonte <gi...@git.apache.org>.
Github user VenturaDelMonte commented on the issue:

    https://github.com/apache/flink/pull/3285
  
    Sure, I can take care of the all-window case!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFunction

Posted by VenturaDelMonte <gi...@git.apache.org>.
Github user VenturaDelMonte commented on the issue:

    https://github.com/apache/flink/pull/3285
  
    I checked the code and it looks fine according to me. I see you are dealing with RichFunction inheritance in wrapper classes. It is something that bothered me too when I was working on the previous PR. Actually there could be a similar minor issue in RichProcessWindowFunction due to duplicate code, however I cannot see any easy workaround for it. 
    Regarding the overall feature, are you planning to do anything similar for #2946 or should I reflect these changes also there? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3285: [FLINK-4997] [streaming] Introduce ProcessWindowFunction

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:

    https://github.com/apache/flink/pull/3285
  
    @VenturaDelMonte I opened this: https://issues.apache.org/jira/browse/FLINK-5740. I want to make `WrappingFunction` an interface because, as you noted, `RichProcessWindowFunction` does currently not work.
    
    I would merge this then. If you want you can pick up the work on the all-window case again and carry it to an end in the same way. What do you think?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---