You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tzulitai <gi...@git.apache.org> on 2017/08/18 13:29:19 UTC

[GitHub] flink pull request #4565: [FLINK-7429] [kinesis] Add migration test coverage...

GitHub user tzulitai opened a pull request:

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

    [FLINK-7429] [kinesis] Add migration test coverage for Flink 1.2 and 1.3

    ## What is the purpose of the change
    
    The migration tests for the Kinesis consumer did not cover Flink 1.2 and 1.3. This pull request fixes that.
    
    It also fixes a minor bug, where restoring from empty state had different behaviours across different Flink versions, that was discovered as part of additionally having these new migration tests.
    
    ## Brief change log
    
    - (1st commit) generalize `FlinkKinesisConsumerMigrationTest` to cover all Flink versions & add stored savepoints for Flink 1.2 and 1.3.
    - (2nd commit) Fix different restore behaviour when restoring empty state.
    
    ## Verifying this change
    
    Previous behaviours should be covered by existing tests.
    The additional tests in `FlinkKinesisConsumerMigrationTest` are simply an extra guard that we were doing things correctly.
    
    ## 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)`: **no**
      - The serializers: **no**
      - The runtime per-record code paths (performance sensitive): **no**
      - Anything that affects deployment or recovery: **no**
    
    ## Documentation
    
      - Does this pull request introduce a new feature? **no**
      - If yes, how is the feature documented? **not applicable**
    


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

    $ git pull https://github.com/tzulitai/flink FLINK-7429

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

    https://github.com/apache/flink/pull/4565.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 #4565
    
----
commit 04e73db786eb2cc7129ea6a8fbe05ca156ef3bce
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date:   2017-08-18T03:27:38Z

    [FLINK-7429] [kinesis] Add IT tests for migration from 1.2 / 1.3

commit 47fbdf7495a95682a4da69a5e9a060cdf4c496b3
Author: Tzu-Li (Gordon) Tai <tz...@apache.org>
Date:   2017-08-18T13:17:27Z

    [FLINK-7429] [kinesis] Unify empty state restore behaviour across 1.1 / 1.2 / 1.3
    
    Prior to this commit, when restoring empty state from previous Flink
    versions, the behaviour was different for each version. For older
    versions, restoring empty state results in `null`. For newer versions,
    restoring empty state results in an empty map.
    
    We want that an empty map represents "this is a restored run, but there
    was no state for us", and a null to represent" this is not a restored
    run".

----


---
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 #4565: [FLINK-7429] [kinesis] Add migration test coverage for Fl...

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

    https://github.com/apache/flink/pull/4565
  
    I merged this on master, waiting for tests to pass on release-1.3. 😃 
    
    Could you please close this PR?


---
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 #4565: [FLINK-7429] [kinesis] Add migration test coverage...

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

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


---

[GitHub] flink issue #4565: [FLINK-7429] [kinesis] Add migration test coverage for Fl...

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

    https://github.com/apache/flink/pull/4565
  
    Checkstyle is complaining about `FlinkKinesisConsumerMigrationTest`. I fixed and pushed on travis. 
    
    The changes look very good. 👍 I'll merge when Travis is green.


---
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 #4565: [FLINK-7429] [kinesis] Add migration test coverage for Fl...

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

    https://github.com/apache/flink/pull/4565
  
    @aljoscha thanks! Closing ..


---