[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...

[zentol]

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5364#discussion_r165459894
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/keyed/KeyedJob.java
 ---
@@ -100,9 +100,7 @@ public static void main(String[] args) throws Exception 
{
.map(new StatefulStringStoringMap(mode, "first"))
.setParallelism(4);
 
-   if (mode == ExecutionMode.MIGRATE || mode == 
ExecutionMode.RESTORE) {
--- End diff --

yeah it should be alright to remove that, but let's chain the uid call to 
the operator creation as we do for the others for style points.


---

[GitHub] flink pull request #5364: [FLINK-8472] [test] Extend all migration tests for...

[tzulitai]

Github user tzulitai commented on a diff in the pull request:

https://github.com/apache/flink/pull/5364#discussion_r165385564
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/keyed/KeyedJob.java
 ---
@@ -100,9 +100,7 @@ public static void main(String[] args) throws Exception 
{
.map(new StatefulStringStoringMap(mode, "first"))
.setParallelism(4);
 
-   if (mode == ExecutionMode.MIGRATE || mode == 
ExecutionMode.RESTORE) {
--- End diff --

@zentol can you confirm whether these changes make sense?

From the discussions I see in https://github.com/apache/flink/pull/3844, I 
assume this was a leftover to write a 1.2 savepoint (when uids couldn't be 
added for each chained operator separately). So it should be ok to remove this?


---