You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/03 18:46:31 UTC

[GitHub] [flink-connector-elasticsearch] zentol commented on a change in pull request #4: [FLINK-26958][BuildSystem] Add Github Actions build for flink-connector-elasticsearch

zentol commented on a change in pull request #4:
URL: https://github.com/apache/flink-connector-elasticsearch/pull/4#discussion_r841261516



##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12

Review comment:
       Surely you could rid of the whoe scala version business and just hard-code whatever scala version you want to use during testing.

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:

Review comment:
       This job is a bit pointless since the compiled files aren't used in the test job.
   I'd just model the test execution as another step of this job.

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '8'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5
+        uses: stCarolas/setup-maven@v4.2

Review comment:
       have you double-checked that this action is allowed in apache projects? (INFRA maintains an allow list)

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2

Review comment:
       I'm not sure if this fragmentation really makes things easier to understand.

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '8'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5

Review comment:
       Just FYI this restriction will likely no longer apply since shading should not occur in intermediate modules.

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '8'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5
+        uses: stCarolas/setup-maven@v4.2
+        with:
+          maven-version: 3.2.5
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Compile flink-connector-elasticsearch
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  test_ci_jdk8:
+    needs: compile_ci_jdk8
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Dscala-2.12
+      MVN_TEST_OPTIONS: -B --no-snapshot-updates
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Test - connectors
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  compile_ci_jdk11:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '11'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5
+        uses: stCarolas/setup-maven@v4.2
+        with:
+          maven-version: 3.2.5
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Compile flink-connector-elasticsearch
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  test_ci_jdk11:
+    needs: compile_ci_jdk11

Review comment:
       jdk switches could be easily modeled as a matrix.
   
   this job is also not using the correct jdk nor maven version.

##########
File path: .github/workflows/ci.yml
##########
@@ -0,0 +1,141 @@
+################################################################################
+#  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.
+################################################################################
+
+name: Build flink-connector-elasticsearch
+on: [push, pull_request]
+jobs:
+  compile_ci_jdk8:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '8'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5
+        uses: stCarolas/setup-maven@v4.2
+        with:
+          maven-version: 3.2.5
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Compile flink-connector-elasticsearch
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  test_ci_jdk8:
+    needs: compile_ci_jdk8
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Dscala-2.12
+      MVN_TEST_OPTIONS: -B --no-snapshot-updates
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Test - connectors
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  compile_ci_jdk11:
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Pcheck-convergence -Dscala-2.12
+      MVN_TEST_OPTIONS: -Dmaven.javadoc.skip=true -DskipTests -Dfast -U -B
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      #      Temporary disable convergence phase since that will fail
+      #      MVN_COMMON_OPTIONS: -Dflink.convergence.phase=install -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v2
+
+      - name: Set JDK
+        uses: actions/setup-java@v2
+        with:
+          java-version: '11'
+          distribution: 'adopt'
+
+      - name: Set Maven 3.2.5
+        uses: stCarolas/setup-maven@v4.2
+        with:
+          maven-version: 3.2.5
+
+      - name: Cache local Maven repository
+        uses: actions/cache@v3
+        with:
+          path: ~/.m2/repository
+          key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }}
+          restore-keys: |
+            ${{ runner.os }}-maven-
+
+      - name: Compile flink-connector-elasticsearch
+        run: mvn ${{ env.MVN_GOALS }} ${{ env.MVN_PROFILES }} ${{ env.MVN_TEST_OPTIONS }} ${{ env.MVN_COMMON_OPTIONS }} ${{ env.MVN_CONNECTION_OPTIONS }}
+
+  test_ci_jdk11:
+    needs: compile_ci_jdk11
+    runs-on: ubuntu-latest
+    env:
+      MVN_GOALS: clean install
+      MVN_PROFILES: -Dscala-2.12
+      MVN_TEST_OPTIONS: -B --no-snapshot-updates
+      MVN_COMMON_OPTIONS: -Dflink.forkCount=2 -Dflink.forkCountTestPackage=2
+      MVN_CONNECTION_OPTIONS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.httpconnectionManager.ttlSeconds=120

Review comment:
       likely unnecessary. You could also consider running everything in a docker container, and bake all the required dependencies into said image. Then you don't need to worry about maven connectivity or maven caches for that matter. Could be overkill though 🤷 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org