You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2015/10/28 11:45:09 UTC

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

GitHub user zentol opened a pull request:

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

    [FLINK-2901] Remove Record API dependencies from flink-tests

    This PR removes all dependencies to the Record API in the flink-tests project.
    
    For this, tests were either ported or removed.
    
    The following files were NOT ported:
    * recordJobs/*
     * except kmeans/udfs CoordVector, PointInFormat (moved & ported)
     * except util/InfiniteIntegerInputFormat (moved & ported)
    * recordJobTests/*
    * operators/*
     *  except CoGroupSortITCase, MapPartitionITCase, ObjectReuseITCase (moved)
    * accumulators/AccumulatorIterativeITCase
     * Unfinished test, should be addressed in a separate issue
    * IterationTerminationWithTwoTails
     * nigh identical with IterationTerminationWithTerminationTail (ported version also failed)
    * iterative/DeltaPageRankITCase, IterativeKMeansITCase, KMeansITCase
     * behaved like they belong into /recordJobTests/ (as in they simply execute a recordJob), thus removed
    * optimizer/iterations/IterativeKMeansITCase
     * overlaps with ConnectedComponentsITCase (need a second opinion on this!)
    * util/FailingTestBase
     * integrated into TaskFailureITCase (only user of the class)
    * testPrograms/util/tests/*
    
    cancelling/MatchJoinCancellingITCase was ported but disabled since it is unstable, and also disabled before. Separate issue seems appropriate.

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

    $ git pull https://github.com/zentol/flink 2901_record_tests

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

    https://github.com/apache/flink/pull/1306.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 #1306
    
----
commit ae3e5d64240feb0f2d9f5b74b709567ee3c66ec4
Author: Fabian Hueske <fh...@apache.org>
Date:   2015-10-22T19:13:59Z

    Remove ITCases for Record API operators

commit 88fcd3419774a48bc42c21fc46a745129bc4c0ac
Author: Fabian Hueske <fh...@apache.org>
Date:   2015-10-22T19:15:38Z

    Move operator ITCases into correct package

commit 4b9cecbe747d1131849070223821cbb4446a2a48
Author: Fabian Hueske <fh...@apache.org>
Date:   2015-10-22T19:18:04Z

    Remove Record API dependencies from CC iteration tests

commit 75052028dc8f29b4280d9c29d61fcb6f6c00ff0a
Author: Fabian Hueske <fh...@apache.org>
Date:   2015-10-22T19:19:34Z

    Remove Record API dependencies from WordCount compiler test

commit 91ca56c0df8a07cd142fd764b13af6ea3d7dae8a
Author: Fabian Hueske <fh...@apache.org>
Date:   2015-10-22T19:20:34Z

    Remove Record API program tests

commit 491afa1ea220f5292c36910d5c4f0c2491edc859
Author: zentol <s....@web.de>
Date:   2015-10-25T14:07:37Z

    Remove deprecation warning suppressions

commit a32a0d1b33c7ffcd4a997fa1cdd11fe299bc7e60
Author: zentol <s....@web.de>
Date:   2015-10-27T20:22:38Z

    [FLINK-2901] Remove Record API dependencies from flink-tests #1

commit 0600da3154665aaf1ee02510e24f8222ca12af5f
Author: zentol <s....@web.de>
Date:   2015-10-27T20:22:45Z

    [FLINK-2901] Remove Record API dependencies from flink-tests #2

----


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-159235437
  
    Go ahead, i won't have time for it today anyway :)


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-159223361
  
    Hi @zentol, sorry for the delayed review.
    You are right, `DeltaPageRankITCase` (which executes for some reason the Record API delta iteration ConnectedComponents example) is identical to the DataSet `ConnectedComponentsITCase`. So we do not port this one.
    I also agree to remove the checks for empty arguments in the `PreviewPlanDumpTest`. This requires cooperation of the job, i.e., the job may not fail for empty arguments, and tests exactly the same code paths as the tests with arguments, as far as I see.
    
    However, I do not think that the `ClassLoaderITCase` which calls the `KMeansForTest` is a good replacement for the `IterativeKMeansITCase`. The programs are identical but, the `ClassLoaderITCase` only checks if the program executes without an error and does not check the result.
    
    @zentol Is it OK for you if I add this one last test and merge this PR afterwards?


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44392775
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterationTerminationWithTwoTails.java ---
    @@ -1,134 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.test.iterative;
    -
    -import java.io.Serializable;
    -import java.util.Iterator;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.api.java.record.functions.MapFunction;
    -import org.apache.flink.api.java.record.functions.ReduceFunction;
    -import org.apache.flink.api.java.record.io.CsvOutputFormat;
    -import org.apache.flink.api.java.record.io.TextInputFormat;
    -import org.apache.flink.api.java.record.operators.BulkIteration;
    -import org.apache.flink.api.java.record.operators.FileDataSink;
    -import org.apache.flink.api.java.record.operators.FileDataSource;
    -import org.apache.flink.api.java.record.operators.MapOperator;
    -import org.apache.flink.api.java.record.operators.ReduceOperator;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -import org.apache.flink.types.Record;
    -import org.apache.flink.types.StringValue;
    -import org.apache.flink.util.Collector;
    -import org.junit.Assert;
    -
    -@SuppressWarnings("deprecation")
    -public class IterationTerminationWithTwoTails extends RecordAPITestBase {
    -
    -	private static final String INPUT = "1\n" + "2\n" + "3\n" + "4\n" + "5\n";
    -	private static final String EXPECTED = "22\n";
    -
    -	protected String dataPath;
    -	protected String resultPath;
    -
    -	public IterationTerminationWithTwoTails(){
    -		setTaskManagerNumSlots(parallelism);
    -	}
    -
    -	@Override
    -	protected void preSubmit() throws Exception {
    -		dataPath = createTempFile("datapoints.txt", INPUT);
    -		resultPath = getTempFilePath("result");
    -	}
    -	
    -	@Override
    -	protected void postSubmit() throws Exception {
    -		compareResultsByLinesInMemory(EXPECTED, resultPath);
    -	}
    -
    -	@Override
    -	protected Plan getTestJob() {
    -		return getTestPlanPlan(parallelism, dataPath, resultPath);
    -	}
    -	
    -	private static Plan getTestPlanPlan(int numSubTasks, String input, String output) {
    -
    -		FileDataSource initialInput = new FileDataSource(TextInputFormat.class, input, "input");
    -		
    -		BulkIteration iteration = new BulkIteration("Loop");
    -		iteration.setInput(initialInput);
    -		iteration.setMaximumNumberOfIterations(5);
    -		Assert.assertTrue(iteration.getMaximumNumberOfIterations() > 1);
    -
    -		ReduceOperator sumReduce = ReduceOperator.builder(new SumReducer())
    -				.input(iteration.getPartialSolution())
    -				.name("Compute sum (Reduce)")
    -				.build();
    -		
    -		iteration.setNextPartialSolution(sumReduce);
    -		
    -		MapOperator terminationMapper = MapOperator.builder(new TerminationMapper())
    -				.input(iteration.getPartialSolution())
    --- End diff --
    
    so THAT was the difference, will port it!


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44393132
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterativeKMeansITCase.java ---
    @@ -1,62 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -
    -package org.apache.flink.test.iterative;
    -
    -import java.util.ArrayList;
    -import java.util.List;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.test.recordJobs.kmeans.KMeansBroadcast;
    -import org.apache.flink.test.testdata.KMeansData;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -
    -
    -public class IterativeKMeansITCase extends RecordAPITestBase {
    --- End diff --
    
    does the KMeansForTest qualify for this?


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44386994
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterationWithAllReducerITCase.java ---
    @@ -18,91 +18,34 @@
     
     package org.apache.flink.test.iterative;
     
    -import java.io.Serializable;
    -import java.util.Iterator;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.api.java.record.functions.ReduceFunction;
    -import org.apache.flink.api.java.record.io.CsvOutputFormat;
    -import org.apache.flink.api.java.record.io.TextInputFormat;
    -import org.apache.flink.api.java.record.operators.BulkIteration;
    -import org.apache.flink.api.java.record.operators.FileDataSink;
    -import org.apache.flink.api.java.record.operators.FileDataSource;
    -import org.apache.flink.api.java.record.operators.ReduceOperator;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -import org.apache.flink.types.Record;
    -import org.apache.flink.types.StringValue;
    -import org.apache.flink.util.Collector;
    -import org.junit.Assert;
    -
    -@SuppressWarnings("deprecation")
    -public class IterationWithAllReducerITCase extends RecordAPITestBase {
    -
    -	private static final String INPUT = "1\n" + "1\n" + "1\n" + "1\n" + "1\n" + "1\n" + "1\n" + "1\n";
    +import java.util.List;
    +import org.apache.flink.api.common.functions.ReduceFunction;
    +import org.apache.flink.api.java.DataSet;
    +import org.apache.flink.api.java.ExecutionEnvironment;
    +import org.apache.flink.api.java.operators.IterativeDataSet;
    +import org.apache.flink.test.util.JavaProgramTestBase;
    +
    +public class IterationWithAllReducerITCase extends JavaProgramTestBase {
     	private static final String EXPECTED = "1\n";
     
    -	protected String dataPath;
    -	protected String resultPath;
    -
    -	public IterationWithAllReducerITCase(){
    -		setTaskManagerNumSlots(4);
    -	}
    -
     	@Override
    -	protected void preSubmit() throws Exception {
    -		dataPath = createTempFile("datapoints.txt", INPUT);
    -		resultPath = getTempFilePath("result");
    -	}
    -	
    -	@Override
    -	protected void postSubmit() throws Exception {
    -		compareResultsByLinesInMemory(EXPECTED, resultPath);
    -	}
    -
    -	@Override
    -	protected Plan getTestJob() {
    -		Plan plan = getTestPlanPlan(parallelism, dataPath, resultPath);
    -		return plan;
    -	}
    +	protected void testProgram() throws Exception {
    +		ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment();
    +		env.setParallelism(4);
     
    -	
    -	private static Plan getTestPlanPlan(int numSubTasks, String input, String output) {
    +		DataSet<String> initialInput = env.fromElements("1", "1", "1", "1", "1", "1", "1", "1");
     
    -		FileDataSource initialInput = new FileDataSource(TextInputFormat.class, input, "input");
    -		
    -		BulkIteration iteration = new BulkIteration("Loop");
    -		iteration.setInput(initialInput);
    -		iteration.setMaximumNumberOfIterations(5);
    -		
    -		Assert.assertTrue(iteration.getMaximumNumberOfIterations() > 1);
    +		IterativeDataSet<String> iteration = initialInput.iterate(5).name("Loop");
     
    -		ReduceOperator sumReduce = ReduceOperator.builder(new PickOneReducer())
    -				.input(iteration.getPartialSolution())
    -				.name("Compute sum (Reduce)")
    -				.build();
    -		
    -		iteration.setNextPartialSolution(sumReduce);
    +		DataSet<String> sumReduce = iteration.reduce(new ReduceFunction<String>(){
    --- End diff --
    
    A `GroupReduceFunction` would be closer to the original test.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44386181
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterationTerminationWithTwoTails.java ---
    @@ -1,134 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.test.iterative;
    -
    -import java.io.Serializable;
    -import java.util.Iterator;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.api.java.record.functions.MapFunction;
    -import org.apache.flink.api.java.record.functions.ReduceFunction;
    -import org.apache.flink.api.java.record.io.CsvOutputFormat;
    -import org.apache.flink.api.java.record.io.TextInputFormat;
    -import org.apache.flink.api.java.record.operators.BulkIteration;
    -import org.apache.flink.api.java.record.operators.FileDataSink;
    -import org.apache.flink.api.java.record.operators.FileDataSource;
    -import org.apache.flink.api.java.record.operators.MapOperator;
    -import org.apache.flink.api.java.record.operators.ReduceOperator;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -import org.apache.flink.types.Record;
    -import org.apache.flink.types.StringValue;
    -import org.apache.flink.util.Collector;
    -import org.junit.Assert;
    -
    -@SuppressWarnings("deprecation")
    -public class IterationTerminationWithTwoTails extends RecordAPITestBase {
    -
    -	private static final String INPUT = "1\n" + "2\n" + "3\n" + "4\n" + "5\n";
    -	private static final String EXPECTED = "22\n";
    -
    -	protected String dataPath;
    -	protected String resultPath;
    -
    -	public IterationTerminationWithTwoTails(){
    -		setTaskManagerNumSlots(parallelism);
    -	}
    -
    -	@Override
    -	protected void preSubmit() throws Exception {
    -		dataPath = createTempFile("datapoints.txt", INPUT);
    -		resultPath = getTempFilePath("result");
    -	}
    -	
    -	@Override
    -	protected void postSubmit() throws Exception {
    -		compareResultsByLinesInMemory(EXPECTED, resultPath);
    -	}
    -
    -	@Override
    -	protected Plan getTestJob() {
    -		return getTestPlanPlan(parallelism, dataPath, resultPath);
    -	}
    -	
    -	private static Plan getTestPlanPlan(int numSubTasks, String input, String output) {
    -
    -		FileDataSource initialInput = new FileDataSource(TextInputFormat.class, input, "input");
    -		
    -		BulkIteration iteration = new BulkIteration("Loop");
    -		iteration.setInput(initialInput);
    -		iteration.setMaximumNumberOfIterations(5);
    -		Assert.assertTrue(iteration.getMaximumNumberOfIterations() > 1);
    -
    -		ReduceOperator sumReduce = ReduceOperator.builder(new SumReducer())
    -				.input(iteration.getPartialSolution())
    -				.name("Compute sum (Reduce)")
    -				.build();
    -		
    -		iteration.setNextPartialSolution(sumReduce);
    -		
    -		MapOperator terminationMapper = MapOperator.builder(new TerminationMapper())
    -				.input(iteration.getPartialSolution())
    --- End diff --
    
    The difference to `IterationTerminationWithTerminationTail` is that the input of the `terminationMapper` is the partial solution and not the result of the `SumReducer`, right?


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-159249074
  
    Thanks, I'll port the last test and merge this PR


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-151800406
  
    Nice @zentol! Thanks for this effort!


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44388274
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterativeKMeansITCase.java ---
    @@ -1,62 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -
    -package org.apache.flink.test.iterative;
    -
    -import java.util.ArrayList;
    -import java.util.List;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.test.recordJobs.kmeans.KMeansBroadcast;
    -import org.apache.flink.test.testdata.KMeansData;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -
    -
    -public class IterativeKMeansITCase extends RecordAPITestBase {
    --- End diff --
    
    We should check if there is another test in the test suite that uses a broadcast variable in a bulk iteration. 
    If not, we should implement such a test (not necessarily reimplementing the KMeans program).


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-155393090
  
    Hi @zentol, thanks again for the huge effort to port all these tests!
    
    Looks mostly good, except for some minor things and three tests which should be ported to preserve test coverage, IMO.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1306#discussion_r44386043
  
    --- Diff: flink-tests/src/test/java/org/apache/flink/test/iterative/IterationTerminationWithTwoTails.java ---
    @@ -1,134 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one
    - * or more contributor license agreements.  See the NOTICE file
    - * distributed with this work for additional information
    - * regarding copyright ownership.  The ASF licenses this file
    - * to you under the Apache License, Version 2.0 (the
    - * "License"); you may not use this file except in compliance
    - * with the License.  You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.test.iterative;
    -
    -import java.io.Serializable;
    -import java.util.Iterator;
    -
    -import org.apache.flink.api.common.Plan;
    -import org.apache.flink.api.java.record.functions.MapFunction;
    -import org.apache.flink.api.java.record.functions.ReduceFunction;
    -import org.apache.flink.api.java.record.io.CsvOutputFormat;
    -import org.apache.flink.api.java.record.io.TextInputFormat;
    -import org.apache.flink.api.java.record.operators.BulkIteration;
    -import org.apache.flink.api.java.record.operators.FileDataSink;
    -import org.apache.flink.api.java.record.operators.FileDataSource;
    -import org.apache.flink.api.java.record.operators.MapOperator;
    -import org.apache.flink.api.java.record.operators.ReduceOperator;
    -import org.apache.flink.test.util.RecordAPITestBase;
    -import org.apache.flink.types.Record;
    -import org.apache.flink.types.StringValue;
    -import org.apache.flink.util.Collector;
    -import org.junit.Assert;
    -
    -@SuppressWarnings("deprecation")
    -public class IterationTerminationWithTwoTails extends RecordAPITestBase {
    --- End diff --
    
    I think we should keep this test and port it.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2901] Remove Record API dependencies fr...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the pull request:

    https://github.com/apache/flink/pull/1306#issuecomment-157382048
  
    @fhueske I've addressed most of your concerns.
    
    Things that still need work / clarification:
    * PreviewPlanDumpTest was previously executed with 2 different sets of arguments, now only with 1. Should this be changed back to the previous behaviour? The arguments affect paths for sources/sink, parallelism and the number of Iterations
    * test.classloading.jar.KMeansForTest appears to be a good replacement for the IterativeKMeansITCase, what's your take on that?
    * The removed delta ilteration PageRank program looks very similar to the ConnectedComponents implementation under flink-examples. I don't think this needs a separate port.


---
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 and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---