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

Validation error in Kinesis Consumer when using AT_TIMESTAMP as start position

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: Kinesis Connector
    • Labels:
      None

      Description

      private static void validateOptionalDateProperty(Properties config, String key, String message) {
          if (config.containsKey(key)) {
              try {
                  initTimestampDateFormat.parse(config.getProperty(key)); —
                  double value = Double.parseDouble(config.getProperty(key)); —
                  if (value < 0) { throw new NumberFormatException(); }
              } catch (ParseException | NumberFormatException e){
                  throw new IllegalArgumentException(message); }
              }
          }
      }
      

      This validation function will always fail regardless of either string format or double type.

        Issue Links

          Activity

          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited

          Thanks for looking into this. I would make this a blocker for 1.2.1, as it was reported by a user and basically makes timestamp start positions with the Kinesis consumer unusable.

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - - edited Thanks for looking into this. I would make this a blocker for 1.2.1, as it was reported by a user and basically makes timestamp start positions with the Kinesis consumer unusable.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user tony810430 opened a pull request:

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

          FLINK-6211 [kinesis] Validation error in Kinesis Consumer when using AT_TIMESTAMP as start position

          Address the bug in validation function to check if the date string is well formed when using AT_TIMESTAMP as start position.

          This function needs to check if one of the two conditions is passed, but I made a mistake to check all conditions should be passed. That causes the problem.

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

          $ git pull https://github.com/tony810430/flink FLINK-6211

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

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


          commit cb14ddcffdcbbd6fc84619aab475d7b9ff6d1644
          Author: Tony Wei <tony19920430@gmail.com>
          Date: 2017-03-29T07:08:04Z

          FLINK-6211 Validation error in Kinesis Consumer when using AT_TIMESTAMP as start position


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user tony810430 opened a pull request: https://github.com/apache/flink/pull/3637 FLINK-6211 [kinesis] Validation error in Kinesis Consumer when using AT_TIMESTAMP as start position Address the bug in validation function to check if the date string is well formed when using AT_TIMESTAMP as start position. This function needs to check if one of the two conditions is passed, but I made a mistake to check all conditions should be passed. That causes the problem. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tony810430/flink FLINK-6211 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3637.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 #3637 commit cb14ddcffdcbbd6fc84619aab475d7b9ff6d1644 Author: Tony Wei <tony19920430@gmail.com> Date: 2017-03-29T07:08:04Z FLINK-6211 Validation error in Kinesis Consumer when using AT_TIMESTAMP as start position
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tony810430 commented on the issue:

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

          Hi @tsriharsha,

          Sorry for missing this PR and pushing my patch https://github.com/apache/flink/pull/3637.

          I have created a JIRA task (FLINK-6211(https://issues.apache.org/jira/browse/FLINK-6211)) for it. If you don't mind to update the title and add some tests for this patch, I could close my PR and let you continue working on it.

          Thank you.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tony810430 commented on the issue: https://github.com/apache/flink/pull/3636 Hi @tsriharsha, Sorry for missing this PR and pushing my patch https://github.com/apache/flink/pull/3637 . I have created a JIRA task ( FLINK-6211 ( https://issues.apache.org/jira/browse/FLINK-6211 )) for it. If you don't mind to update the title and add some tests for this patch, I could close my PR and let you continue working on it. Thank you.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          Hi @tsriharsha!

          First of all, thanks a lot for reporting this bug. Contributions in Flink, however, require a corresponding JIRA ticket. In the future, I would recommend always opening a JIRA before the pull request so that other contributors will know that the issue is worked on already (and would therefore avoid race conditions / duplicate work like right now :-D).

          Generally, the implementation #3637 is cleaner and includes tests for the fix. I think I'll proceed with FLINK-6211 using the patch in #3637. Hope you understand, and welcome more contributions!

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3636 Hi @tsriharsha! First of all, thanks a lot for reporting this bug. Contributions in Flink, however, require a corresponding JIRA ticket. In the future, I would recommend always opening a JIRA before the pull request so that other contributors will know that the issue is worked on already (and would therefore avoid race conditions / duplicate work like right now :-D). Generally, the implementation #3637 is cleaner and includes tests for the fix. I think I'll proceed with FLINK-6211 using the patch in #3637. Hope you understand, and welcome more contributions!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tzulitai commented on the issue:

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

          LGTM, thanks a lot for the fix and tests @tony810430! Merging this to `master` and `release-1.2`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user tzulitai commented on the issue: https://github.com/apache/flink/pull/3637 LGTM, thanks a lot for the fix and tests @tony810430! Merging this to `master` and `release-1.2`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/3637
          Hide
          tzulitai Tzu-Li (Gordon) Tai added a comment -

          Thanks for the contributions!

          Resolved for 1.3.0 with http://git-wip-us.apache.org/repos/asf/flink/commit/69843fe.

          Show
          tzulitai Tzu-Li (Gordon) Tai added a comment - Thanks for the contributions! Resolved for 1.3.0 with http://git-wip-us.apache.org/repos/asf/flink/commit/69843fe .

            People

            • Assignee:
              tonywei Wei-Che Wei
              Reporter:
              tonywei Wei-Che Wei
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development