Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-6164

Make ProcessWindowFunction a RichFunction

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0, 1.4.0
    • Component/s: DataStream API
    • Labels:
      None

      Description

      ProcessWindowFunction is an abstract class so we can make it a RichFunction by default and remove RichProcessWindowFunction. This is in line with ProcessFunction which is also a RichFunction.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user zentol opened a pull request:

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

          FLINK-6164 Make ProcessWindowFunction a RichFunction

          This PR modifies the ProcessWindowFunction and ProcessAllWindowFunction classes to extend AbstractRichtFunction.

          The existing Rich* versions were deprecated, and all usages of them replaced.

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

          $ git pull https://github.com/zentol/flink 6164_rich_process

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

          https://github.com/apache/flink/pull/3824.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 #3824


          commit 7991d4d4f2963fe5e60a9ccca5a606b41b97c962
          Author: zentol <chesnay@apache.org>
          Date: 2017-05-03T13:57:05Z

          FLINK-6164 Make ProcessWindowFunction a RichFunction


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/3824 FLINK-6164 Make ProcessWindowFunction a RichFunction This PR modifies the ProcessWindowFunction and ProcessAllWindowFunction classes to extend AbstractRichtFunction. The existing Rich* versions were deprecated, and all usages of them replaced. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 6164_rich_process Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3824.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 #3824 commit 7991d4d4f2963fe5e60a9ccca5a606b41b97c962 Author: zentol <chesnay@apache.org> Date: 2017-05-03T13:57:05Z FLINK-6164 Make ProcessWindowFunction a RichFunction
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3824#discussion_r114771220

          — Diff: flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/function/ProcessWindowFunction.scala —
          @@ -36,7 +36,10 @@ import org.apache.flink.util.Collector

          • @tparam W The type of the window.
            */
            @PublicEvolving
            -abstract class ProcessWindowFunction[IN, OUT, KEY, W <: Window] extends Function with Serializable {
            +abstract class ProcessWindowFunction[IN, OUT, KEY, W <: Window]
              • End diff –

          See above.

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/3824#discussion_r114771220 — Diff: flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/function/ProcessWindowFunction.scala — @@ -36,7 +36,10 @@ import org.apache.flink.util.Collector @tparam W The type of the window. */ @PublicEvolving -abstract class ProcessWindowFunction [IN, OUT, KEY, W <: Window] extends Function with Serializable { +abstract class ProcessWindowFunction [IN, OUT, KEY, W <: Window] End diff – See above.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3824#discussion_r114771179

          — Diff: flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/function/ProcessAllWindowFunction.scala —
          @@ -35,7 +35,9 @@ import org.apache.flink.util.Collector

          • @tparam W The type of the window.
            */
            @PublicEvolving
            -abstract class ProcessAllWindowFunction[IN, OUT, W <: Window] extends Function with Serializable {
            +abstract class ProcessAllWindowFunction[IN, OUT, W <: Window]
              • End diff –

          I think the `Serializable` is not needed here. (Was there before, so not introduced in this PR)

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/3824#discussion_r114771179 — Diff: flink-streaming-scala/src/main/scala/org/apache/flink/streaming/api/scala/function/ProcessAllWindowFunction.scala — @@ -35,7 +35,9 @@ import org.apache.flink.util.Collector @tparam W The type of the window. */ @PublicEvolving -abstract class ProcessAllWindowFunction [IN, OUT, W <: Window] extends Function with Serializable { +abstract class ProcessAllWindowFunction [IN, OUT, W <: Window] End diff – I think the `Serializable` is not needed here. (Was there before, so not introduced in this PR)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on a diff in the pull request:

          https://github.com/apache/flink/pull/3824#discussion_r114772035

          — Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/operators/windowing/AccumulatingAlignedProcessingTimeWindowOperatorTest.java —
          @@ -1038,7 +1038,7 @@ private void assertInvalidParameter(long windowSize, long windowSlide) {

          // ------------------------------------------------------------------------

          • private static class StatefulFunction extends RichProcessWindowFunction<Integer, Integer, Integer, TimeWindow> {
            + private static class StatefulFunction extends ProcessWindowFunction<Integer, Integer, Integer, TimeWindow> {
              • End diff –

          With this we add two unused imports.

          And yes, I don't like that we don't have checkstyle active on the tests.

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on a diff in the pull request: https://github.com/apache/flink/pull/3824#discussion_r114772035 — Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/operators/windowing/AccumulatingAlignedProcessingTimeWindowOperatorTest.java — @@ -1038,7 +1038,7 @@ private void assertInvalidParameter(long windowSize, long windowSlide) { // ------------------------------------------------------------------------ private static class StatefulFunction extends RichProcessWindowFunction<Integer, Integer, Integer, TimeWindow> { + private static class StatefulFunction extends ProcessWindowFunction<Integer, Integer, Integer, TimeWindow> { End diff – With this we add two unused imports. And yes, I don't like that we don't have checkstyle active on the tests.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          I've removed Serializable, adjusted the `@Deprecated` annotations like in #3816 and fixed the unused imports (there were also some in `ProcessAllWindowFunction.scala`).

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3824 I've removed Serializable, adjusted the `@Deprecated` annotations like in #3816 and fixed the unused imports (there were also some in `ProcessAllWindowFunction.scala`).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aljoscha commented on the issue:

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

          Ha! I didn't think of the annotations.

          This looks good to merge now!

          Thanks for fixing these so quickly! 😃

          Show
          githubbot ASF GitHub Bot added a comment - Github user aljoscha commented on the issue: https://github.com/apache/flink/pull/3824 Ha! I didn't think of the annotations. This looks good to merge now! Thanks for fixing these so quickly! 😃
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

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

          merging.

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3824 merging.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3824
          Hide
          Zentol Chesnay Schepler added a comment -

          1.3: e76a0aa9e2324c4647509379fe4125d8dc576ff0
          1.4: 5c4560d3bcff47f8563c75ca909d20469e4736c0

          Show
          Zentol Chesnay Schepler added a comment - 1.3: e76a0aa9e2324c4647509379fe4125d8dc576ff0 1.4: 5c4560d3bcff47f8563c75ca909d20469e4736c0

            People

            • Assignee:
              Zentol Chesnay Schepler
              Reporter:
              aljoscha Aljoscha Krettek
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development