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
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
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
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
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
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
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
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
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
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
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
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 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 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
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
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
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 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
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 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?
---
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
21 matches
Mail list logo
