Change to validateOptionalDatePropery method #3636
Conversation
Current version of validateoptionaldateproperty will fail because it will check for date first and if it passes it will fail to get parsed as a double and so it will fail the next step. Please let me know if this is an accurate statement. I just made a small change, may not follow to the pertaining code format though.
|
Hi @tsriharsha, Sorry for missing this PR and pushing my patch #3637. I have created a JIRA task (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. |
| throw new IllegalArgumentException(message); | ||
| } | ||
| } | ||
| } | ||
| public static boolean validateDateFormat(Properties config, String key){ |
There was a problem hiding this comment.
Missing empty line before the method declaration
There was a problem hiding this comment.
I don't think it is necessary to have yet another method for this particular case ...
it can be easily written as 2 catch exception branches, which IMO is easier to understand.
|
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! |
…nesisConsumer This closes apache#3636. This closes apache#3637.
Current version of validateoptionaldateproperty will fail because it will check for date first and if it passes it will fail to get parsed as a double and so it will fail the next step. Please let me know if this is an accurate statement. I just made a small change, may not follow to the pertaining code format though.
Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.
General
Documentation
Tests & Build
mvn clean verifyhas been executed successfully locally or a Travis build has passed