[ 
https://issues.apache.org/jira/browse/GOBBLIN-1365?focusedWorklogId=539609&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-539609
 ]

ASF GitHub Bot logged work on GOBBLIN-1365:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 22/Jan/21 05:48
            Start Date: 22/Jan/21 05:48
    Worklog Time Spent: 10m 
      Work Description: Will-Lo commented on a change in pull request #3206:
URL: https://github.com/apache/incubator-gobblin/pull/3206#discussion_r561360556



##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,162 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:
+    runs-on: ubuntu-latest
+    name: Generate test coverage
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Generate code coverage
+        run: |
+          ./gradlew -PskipTestGroup=disabledOnCI -Dorg.gradle.parallel=false 
-DjacocoBuild=1 $GOBBLIN_GRADLE_OPTS jacocoTestCoverage
+
+  static_checks:
+    name: Run static checks
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Run CheckStyle and FindBugs
+        run: |
+          ./gradlew --no-daemon -x javadoc findbugsMain checkstyleMain 
checkstyleTest checkstyleJmh
+
+  run_tests:
+    timeout-minutes: 60
+    env:
+      GOBBLIN_GRADLE_OPTS: "--no-daemon 
-Dgobblin.metastore.testing.embeddedMysqlEnabled=false 
-PusePreinstalledMysql=true"
+    strategy:
+      matrix:
+        test-group: ["Core Tests", "Service Tests", "Module Tests", "Other 
Tests"]
+      fail-fast: false
+    runs-on: ubuntu-latest
+    needs: build
+    services:
+      mysql:
+        image: mysql:5.7.32
+        env:
+          MYSQL_USER: testUser
+          MYSQL_PASSWORD: testPassword
+          MYSQL_DATABASE: test
+          MYSQL_ROOT_PASSWORD: password
+        ports:
+          - 3306:3306
+        options: --health-cmd="mysqladmin ping" --health-interval=10s 
--health-timeout=5s --health-retries=5
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      - name: Verify mysql connection
+        run: |
+            sudo apt-get install -y mysql-client
+            mysql --host 127.0.0.1 --port 3306 -uroot -ppassword -e "SHOW 
DATABASES"
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Run test group ${{ matrix.test-group }}
+        # Write retry logic as integration tests can fail due to timing out or 
network problems
+        run: |
+          ./gradlew getGroupedTests -PgroupName="${{matrix.test-group}}" > 
temp.txt
+          TASKS=$(sed -n 's/CI Task: //p' temp.txt)
+          echo $TASKS
+
+          n=0
+          until [ "$n" -ge 3 ]
+          do
+            ./gradlew -PskipTestGroup=disabledOnCI $GOBBLIN_GRADLE_OPTS $TASKS 
-Dorg.gradle.parallel=false && break

Review comment:
       Yup, the gradle task automatically returns exit code 1 here and aborts. 
So if it passes it'll call `break`, otherwise it'll be evaluated to false and 
keep looping until it attempts 3 times.

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,162 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:

Review comment:
       So interestingly enough none of the builds need to be dependent on each 
other, because gradle should be building the files needed if it hasn't been 
done so yet on the runner. But I can make the others dependent on `build` too, 
since I think it would make more logical sense

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,162 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:

Review comment:
       So interestingly enough none of the jobs need to be dependent on each 
other because gradle should be building the files needed if it hasn't been done 
so yet on the runner. But I can make the others dependent on `build` too, since 
I think it would make more logical sense

##########
File path: 
gobblin-metastore/src/test/java/org/apache/gobblin/metastore/testing/TestMetastoreDatabaseServer.java
##########
@@ -110,7 +111,7 @@ static Config getDefaultConfig() {
            .put(EMBEDDED_MYSQL_ENABLED_FULL_KEY, true)
            .put(DBUSER_NAME_FULL_KEY, "testUser")
            .put(DBUSER_PASSWORD_FULL_KEY, "testPassword")
-           .put(DBHOST_FULL_KEY, "localhost")
+           .put(DBHOST_FULL_KEY, "127.0.0.1")

Review comment:
       Ah this was left in when I was trying to debug mysql connections I'll 
change it back to localhost.

##########
File path: gradle/scripts/testSetup.gradle
##########
@@ -35,6 +35,7 @@ subprojects {
       }
       testLogging {
         events "started","skipped","passed","failed"
+        exceptionFormat "full"

Review comment:
       There isn't as far as I know, i needed to add this to gain some insight 
on failed tests since I don't think there's a way to access the gradle test 
html after tests.

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,164 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:
+    runs-on: ubuntu-latest
+    name: Generate test coverage
+    needs: build
+    steps:

Review comment:
       Jobs have the ability to be run in parallel, and steps do not. Since the 
test coverage and static checks are independent of the unit tests, it would be 
better to separate them from the normal build process to speed up the pipeline.
   
   I agree that the configs are repeated a lot. It is possible to write 
composite steps in Github actions so that we can reuse them in multiple jobs 
though. 
https://docs.github.com/en/actions/creating-actions/creating-a-composite-run-steps-action.
 I can implement it in this PR or put it in a different PR, depending on how 
high you want to prioritize it.

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,162 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:

Review comment:
       Yup that's a good point, I made the other jobs dependent on the first 
build job so now it should only download dependencies once. The dependencies 
should be stored by the gradle cache after `build` which is shared across jobs, 
but if it's wildcard then gradle might force a dependency download/check. I 
think in this scenario if it causes a problem it's acceptable as the later 
steps would have the newer dependency, and if it causes the tests to break then 
we should really be building against an exact version.

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,162 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:

Review comment:
       Yup that's a good point, I made the other jobs dependent on the first 
build job so now it should only download dependencies once. The dependencies 
should be stored by the gradle cache after `build` which is shared across jobs, 
but if it's wildcard then gradle might force a dependency download/check. I 
think in this scenario if it causes a problem it's acceptable as the later 
steps would have the newer dependency, and if it causes the tests to break then 
we should really be building against an exact version.

##########
File path: .github/workflows/build_and_test.yaml
##########
@@ -0,0 +1,164 @@
+#
+# 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 and Run Tests
+
+on:
+  push:
+    # Publish only on `master`
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+  release:
+    types: [published, edited]
+
+jobs:
+  build:
+    name: Build repository
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the repo
+        uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+      # Stores external dependencies, can be further improved with Gradle 6.1
+      - name: Cache Gradle Dependencies
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          # Only rebuild cache if build.gradle is changed
+          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle
+      - name: Build repository
+        run: |
+          ./gradlew --no-daemon clean build -x test -x javadoc -x findbugsMain 
-x findbugsTest -x checkstyleMain -x checkstyleJmh -x checkstyleTest 
-Dorg.gradle.parallel=true
+
+  test_coverage:
+    runs-on: ubuntu-latest
+    name: Generate test coverage
+    needs: build
+    steps:

Review comment:
       Responding to your first comment,
   
   Yup that's a good point, I made the other jobs dependent on the first build 
job so now it should only download dependencies once. The dependencies should 
be stored by the gradle cache after `build` which is shared across jobs, but if 
it's wildcard then gradle might force a dependency download/check. I think in 
this scenario if it causes a problem it's acceptable as the later steps would 
have the newer dependency, and if it causes the tests to break then we should 
really be building against an exact version.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 539609)
    Time Spent: 3h  (was: 2h 50m)

> Create CI/CD Pipeline on Github Actions from Travis
> ---------------------------------------------------
>
>                 Key: GOBBLIN-1365
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1365
>             Project: Apache Gobblin
>          Issue Type: Improvement
>            Reporter: William Lo
>            Priority: Major
>          Time Spent: 3h
>  Remaining Estimate: 0h
>
> - Travis CI has been extremely slow and flaky recently
> - Due to pricing model changes, builds can be queued for hours
> - Github Actions is free for open source projects and supports multiple 
> builds at once



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to