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

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

GitHub user medcv opened a pull request:

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

    [FLINK-9451]End-to-end test: Scala Quickstarts

    ## What is the purpose of the change
    
    Added an end-to-end test which verifies Flink's quickstarts scala. It does the following:
    
    - create a new Flink project using the quickstarts scala archetype
    - add a new Flink kafka connector dependency to the pom.xml 
    - run mvn clean package
    - verify that no core dependencies are contained in the jar file
    - Run the program
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
      - The serializers: (no)
      - The runtime per-record code paths (performance sensitive): (no)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
      - The S3 file system connector: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)
      - If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)


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

    $ git pull https://github.com/medcv/flink FLINK-9451

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

    https://github.com/apache/flink/pull/6089.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 #6089
    
----
commit 26f5948eff55fd54dbffc3125e0599479bee3bbe
Author: Yadan.JS <y_...@...>
Date:   2018-05-28T16:16:40Z

    [FLINK-9451]End-to-end test: Scala Quickstarts

----


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193581372
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
    +	<dependencies>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +			<scope>provided</scope>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    --- End diff --
    
    Done!


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193581567
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    --- End diff --
    
    Done!


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089
  
    The ES modules are used in `run-nightly-tests.sh`.


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193392825
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
    +	<dependencies>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +			<scope>provided</scope>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    --- End diff --
    
    set `scope` to `provided`


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191738239
  
    --- Diff: flink-end-to-end-tests/test-scripts/elasticsearch-common.sh ---
    @@ -75,6 +76,8 @@ function verify_result {
     }
     
     function shutdown_elasticsearch_cluster {
    +   local index=$1
    --- End diff --
    
    adjust existing usages in `test_streaming_elasticsearch.sh`


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191740715
  
    --- Diff: flink-end-to-end-tests/test-scripts/elasticsearch-common.sh ---
    @@ -75,6 +76,8 @@ function verify_result {
     }
     
     function shutdown_elasticsearch_cluster {
    +   local index=$1
    --- End diff --
    
    +1


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191732783
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -20,27 +20,43 @@ under the License.
     <project xmlns="http://maven.apache.org/POM/4.0.0"
     		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    -
    -	<modelVersion>4.0.0</modelVersion>
    -
     	<parent>
     		<groupId>org.apache.flink</groupId>
     		<artifactId>flink-end-to-end-tests</artifactId>
     		<version>1.6-SNAPSHOT</version>
    -		<relativePath>..</relativePath>
     	</parent>
    +	<modelVersion>4.0.0</modelVersion>
     
    -	<artifactId>flink-elasticsearch5-test</artifactId>
    -	<name>flink-elasticsearch5-test</name>
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
     	<packaging>jar</packaging>
     
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
     	<dependencies>
     		<dependency>
     			<groupId>org.apache.flink</groupId>
     			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
     			<version>${project.version}</version>
     			<scope>provided</scope>
     		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch2_${scala.binary.version}</artifactId>
    --- End diff --
    
    @zentol removed multiple connector versions


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089
  
    Let's not mix concerns here. Re-using examples can lead to situations where the quickstart tests fail because an example was modified, which by all means shouldn't happen.
    The examples should certainly be tested as well though, but that's a separate issue.


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193581591
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
    +	<dependencies>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +			<scope>provided</scope>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch5_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +	</dependencies>
    +
    +	<build>
    --- End diff --
    
    Removed!


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191740903
  
    --- Diff: flink-end-to-end-tests/test-scripts/test_quickstarts.sh ---
    @@ -18,29 +18,38 @@
     ################################################################################
     
     # End to end test for quick starts test.
    +# Usage:
    +# FLINK_DIR=<flink dir> flink-end-to-end-tests/test-scripts/test_quickstarts.sh <TestFileName> <Type (java or scala)>
    --- End diff --
    
    will change the name


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol I reverted back flink-elasticsearch* modules. Do you think we still need them as they've never been used?


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol PR has been updated and usage also changed to 
    `test_quickstarts.sh <Type>`



---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193395032
  
    --- Diff: flink-end-to-end-tests/test-scripts/test_quickstarts.sh ---
    @@ -51,12 +59,12 @@ sed -i -e ''"$(($position + 1))"'i\
     <version>${flink.version}</version>\
    --- End diff --
    
    It would be _awesome_ if you could replace this hard-coded dependency with the one defined in the example project. But I wouldn't block the PR on it.


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

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


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193392673
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    --- End diff --
    
    can be omitted as it is already defined in the root `pom.xml`


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol PR is updated with requested changes! Please review


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol PR is updated!


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191677818
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -86,6 +102,21 @@ under the License.
     					</execution>
     				</executions>
     			</plugin>
    +			<plugin>
    +				<groupId>org.apache.maven.plugins</groupId>
    +				<artifactId>maven-enforcer-plugin</artifactId>
    +				<executions>
    +					<execution>
    +						<id>dependency-convergence</id>
    +						<goals>
    +							<goal>enforce</goal>
    +						</goals>
    +						<configuration>
    +							<skip>true</skip>
    --- End diff --
    
    this was a good hint that what you were trying to do was problematic


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193394392
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -0,0 +1,98 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +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.
    +-->
    +<project xmlns="http://maven.apache.org/POM/4.0.0"
    +		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    +		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    +	<parent>
    +		<groupId>org.apache.flink</groupId>
    +		<artifactId>flink-end-to-end-tests</artifactId>
    +		<version>1.6-SNAPSHOT</version>
    +	</parent>
    +	<modelVersion>4.0.0</modelVersion>
    +
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
    +	<packaging>jar</packaging>
    +
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
    +	<dependencies>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +			<scope>provided</scope>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch5_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +	</dependencies>
    +
    +	<build>
    --- End diff --
    
    This section should be unnecessary.


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol Thanks for review!
    Yes, I totally agree as the most of test script is duplicated, we need to refactor them the way you mentioned, I will update the PR with new changes.
    
    QQ: One reason I borrowed an example from 'Flink-example' module was to e2e test the actual example code also.  One benefit would be testing the actual example and safeguard the 'Flink-example' module from any code changes without passing the tests. (or another cleaner option is to increase unittest coverage to flink-example package).
    Do you think we should have a dedicated module under test folder or extend the 'Flink-example' module with ES example and test e2e for that.


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol Thanks for the review. I made the clean up and did some changes to get the ES dependency from flink-quickstart-test/pom.xml 


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol updated the PR as suggested! Please review


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191733069
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -86,6 +102,21 @@ under the License.
     					</execution>
     				</executions>
     			</plugin>
    +			<plugin>
    +				<groupId>org.apache.maven.plugins</groupId>
    +				<artifactId>maven-enforcer-plugin</artifactId>
    +				<executions>
    +					<execution>
    +						<id>dependency-convergence</id>
    +						<goals>
    +							<goal>enforce</goal>
    +						</goals>
    +						<configuration>
    +							<skip>true</skip>
    --- End diff --
    
    @zentol good point. this is not necessary anymore


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191737346
  
    --- Diff: flink-end-to-end-tests/test-scripts/test_quickstarts.sh ---
    @@ -18,29 +18,38 @@
     ################################################################################
     
     # End to end test for quick starts test.
    +# Usage:
    +# FLINK_DIR=<flink dir> flink-end-to-end-tests/test-scripts/test_quickstarts.sh <TestFileName> <Type (java or scala)>
    --- End diff --
    
    not sure how much value the `TestFileName` adds, given that it's the same for both tests and the package being hard-coded. At the very least it should be called `TestClassName`.


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol sure! I will update the PR with your requested changes.



---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r193581841
  
    --- Diff: flink-end-to-end-tests/test-scripts/test_quickstarts.sh ---
    @@ -51,12 +59,12 @@ sed -i -e ''"$(($position + 1))"'i\
     <version>${flink.version}</version>\
    --- End diff --
    
    I did some code change to get the ES dependency from flink-quickstart-test/pom.xml and removed the hardcoded values


---

[GitHub] flink issue #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

Posted by medcv <gi...@git.apache.org>.
Github user medcv commented on the issue:

    https://github.com/apache/flink/pull/6089
  
    @zentol Thanks! found them :)


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191740667
  
    --- Diff: flink-end-to-end-tests/test-scripts/elasticsearch-common.sh ---
    @@ -56,13 +56,14 @@ function verify_elasticsearch_process_exist {
     
     function verify_result {
         local numRecords=$1
    +    local index=$2
    --- End diff --
    
    +1


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191677160
  
    --- Diff: flink-end-to-end-tests/flink-quickstart-test/pom.xml ---
    @@ -20,27 +20,43 @@ under the License.
     <project xmlns="http://maven.apache.org/POM/4.0.0"
     		 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    -
    -	<modelVersion>4.0.0</modelVersion>
    -
     	<parent>
     		<groupId>org.apache.flink</groupId>
     		<artifactId>flink-end-to-end-tests</artifactId>
     		<version>1.6-SNAPSHOT</version>
    -		<relativePath>..</relativePath>
     	</parent>
    +	<modelVersion>4.0.0</modelVersion>
     
    -	<artifactId>flink-elasticsearch5-test</artifactId>
    -	<name>flink-elasticsearch5-test</name>
    +	<artifactId>flink-quickstart-test</artifactId>
    +	<name>flink-quickstart-test</name>
     	<packaging>jar</packaging>
     
    +	<properties>
    +		<scala.binary.version>2.11</scala.binary.version>
    +	</properties>
    +
     	<dependencies>
     		<dependency>
     			<groupId>org.apache.flink</groupId>
     			<artifactId>flink-streaming-java_${scala.binary.version}</artifactId>
     			<version>${project.version}</version>
     			<scope>provided</scope>
     		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch_${scala.binary.version}</artifactId>
    +			<version>${project.version}</version>
    +		</dependency>
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-connector-elasticsearch2_${scala.binary.version}</artifactId>
    --- End diff --
    
    depending multiple connector versions in one modules is a recipe for dependency conflicts and should be avoided at all costs. There's a reason why the existing examples and connector versions reside in different modules.


---

[GitHub] flink pull request #6089: [FLINK-9451]End-to-end test: Scala Quickstarts

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

    https://github.com/apache/flink/pull/6089#discussion_r191738228
  
    --- Diff: flink-end-to-end-tests/test-scripts/elasticsearch-common.sh ---
    @@ -56,13 +56,14 @@ function verify_elasticsearch_process_exist {
     
     function verify_result {
         local numRecords=$1
    +    local index=$2
    --- End diff --
    
    adjust existing usages in `test_streaming_elasticsearch.sh`


---