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

Add test to fix ordinals of serialized enum StateDescriptor.Type

    Details

      Description

      The order in which elements are contained in the enum StateDescriptor.Type is important because the ordinal is used for custom de/serialization. Changing this order would break the serialization format. We should add a test that enforces the current order.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user StefanRRichter opened a pull request:

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

          FLINK-5442 Ensure order of enum elements in StateDescriptor.Type

          This PR adds a test to ensure the order of elements in enum ``StateDescriptor.Type`` which is important for the serialization format.

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

          $ git pull https://github.com/StefanRRichter/flink fix-descriptor-type-order

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

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


          commit e79b00a2a623dbd06329beee5c81086302821fae
          Author: Stefan Richter <s.richter@data-artisans.com>
          Date: 2017-01-10T21:52:58Z

          FLINK-5442 Ensure order of enum elements in StateDescriptor.Type through test


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user StefanRRichter opened a pull request: https://github.com/apache/flink/pull/3091 FLINK-5442 Ensure order of enum elements in StateDescriptor.Type This PR adds a test to ensure the order of elements in enum ``StateDescriptor.Type`` which is important for the serialization format. You can merge this pull request into a Git repository by running: $ git pull https://github.com/StefanRRichter/flink fix-descriptor-type-order Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3091.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 #3091 commit e79b00a2a623dbd06329beee5c81086302821fae Author: Stefan Richter <s.richter@data-artisans.com> Date: 2017-01-10T21:52:58Z FLINK-5442 Ensure order of enum elements in StateDescriptor.Type through test
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StefanRRichter commented on the issue:

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

          cc @StephanEwen

          Show
          githubbot ASF GitHub Bot added a comment - Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/3091 cc @StephanEwen
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

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

          Looks good, but is that not breaking it now, by changing the order?

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3091 Looks good, but is that not breaking it now, by changing the order?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StefanRRichter commented on the issue:

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

          It only breaks w.r.t. to the previous dev master branch, right? I just preferred to have UNKNOWN at position 0 because the future way of adding enum entries is basically "append only" and having this special state in the middle then would look weird.

          Show
          githubbot ASF GitHub Bot added a comment - Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/3091 It only breaks w.r.t. to the previous dev master branch, right? I just preferred to have UNKNOWN at position 0 because the future way of adding enum entries is basically "append only" and having this special state in the middle then would look weird.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

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

          When was that enum introduced? Can we get a feeling how many people are already using this?

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3091 When was that enum introduced? Can we get a feeling how many people are already using this?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StefanRRichter commented on the issue:

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

          It was introduced by middle/end of december 16. The benefit is also just for esthetic reasons. Would you like me to restore the old order?

          Show
          githubbot ASF GitHub Bot added a comment - Github user StefanRRichter commented on the issue: https://github.com/apache/flink/pull/3091 It was introduced by middle/end of december 16. The benefit is also just for esthetic reasons. Would you like me to restore the old order?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

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

          If it is in so shortly, lets make it clean and keep the new order...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3091 If it is in so shortly, lets make it clean and keep the new order...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

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

          Merging this...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3091 Merging this...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3091
          Hide
          StephanEwen Stephan Ewen added a comment -

          Fixed in

          • 1.2.0 via 758ea79a1cda1dec58d43266487ce663f7205f86
          • 1.3.0 via aa0ef2ac4b053c6bead7ec61a9f7965a907e2ade
          Show
          StephanEwen Stephan Ewen added a comment - Fixed in 1.2.0 via 758ea79a1cda1dec58d43266487ce663f7205f86 1.3.0 via aa0ef2ac4b053c6bead7ec61a9f7965a907e2ade

            People

            • Assignee:
              srichter Stefan Richter
              Reporter:
              srichter Stefan Richter
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development