[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-23 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2342 Builds are passing here: https://travis-ci.org/uce/flink/builds/154236174 I'm going to merge this later today. Thanks for starting this renaming. --- If your project is set up for it, you can r

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-22 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Thank you very much for checking it out. I can see in the branch that you have created you have updated some doc comment that I had added - from 'config' to 'key' and some missing places

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-22 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2342 I had to address some things when reviewing this: https://github.com/uce/flink/tree/ram In general the changes were good, but overlooked many occurrences that are not possible to catch with auto

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-19 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Thanks. No problem. I can wait. --- 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

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-19 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2342 I will check this later today or on Monday. You will have to wait a little, sorry. --- 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

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-19 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Got some time to check this? A gentle reminder !!! --- 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 no

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-18 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Got a green build. Any feedback/reviews here. I know you guys are busy, just a gentle reminder. --- If your project is set up for it, you can reply to this email and have your reply app

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-17 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce I saw that there were conflicts after yesterday's updates to the master. Hence I have rebased my PR and force pushed the changes. Thanks. --- If your project is set up for it, you can r

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-17 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce The test case failures seems to be unrelated. Want to have a look at the updated PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitH

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-17 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Updated the PR with suitable doc updates and also renamed the enum > RecoveryMode to > HighAvailabilityMode --- If your project is set up for it, you can rep

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-16 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 @uce Updated with changes so that all the configs are renamed from 'recovery' to 'high-availability'. --- If your project is set up for it, you can reply to this email and have your reply ap

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-13 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 Ran those two tests individually they seem to pass. But the Rocks_DB version failed due to some initialization error. Thank you @StephanEwen for your comments. --- If your project is set up for

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-13 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/2342 There are some test instabilities that we are trying to address. Seems unrelated to your changes at the first glance. We'll look at the tests again anyways before merging, --- If your project

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-13 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 github build shows two failure > org.apache.flink.test.checkpointing.EventTimeWindowCheckpointingITCase.org.apache.flink.test.checkpointing.EventTimeWindowCheckpointingITCase org.apac

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-12 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 Updated PR request. Handles all other configs and renames them from 'recovery.* to 'high-availability.*'. Also added test case and suitably modified code to ensure that if there are both old a

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-12 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 Just a query, If there are both old and new configs available - we should give priority to the new one right? Even if the value for the old and new configs are different? --- If your proje

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-12 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 > Can you add such a test? Sure I can. The existing test cases helped to ensure that if there are no old configs we are able to fetch from the new config. I can add a test case to ensure

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-12 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2342 Regarding the priority: Yes, I think so. --- 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

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-11 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/2342 I think we need to change more than just that single variable. The other variables should match, e.g. `recovery.jobmanager.port => high-availability.jobmanager.port` and also `recovery.zookeeper.* =>

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-11 Thread StephanEwen
Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/2342 Looks good in general. I think this could use a test case that ensures that the configuration parsing accepts for now both the old and the new config key. Can you add such a test? --- I

[GitHub] flink issue #2342: FLINK-4253 - Rename "recovery.mode" config key to "high-a...

2016-08-09 Thread ramkrish86
Github user ramkrish86 commented on the issue: https://github.com/apache/flink/pull/2342 Looks like only this build failed `https://travis-ci.org/apache/flink/jobs/150992610` and that too due to a cassandra-connector test case. Should be unrelated to this PR. --- If your project