You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@livy.apache.org by "dacort (via GitHub)" <gi...@apache.org> on 2023/03/14 04:39:01 UTC
[GitHub] [incubator-livy] dacort opened a new pull request, #393: Add GitHub Action for CI image
dacort opened a new pull request, #393:
URL: https://github.com/apache/incubator-livy/pull/393
## What changes were proposed in this pull request?
- Adds Several GitHub Workflows
- [ ] CI image builder
- [ ] Unit tests for `git push`
- [ ] Integration tests for PRs
## How was this patch tested?
Manual validation in the course of the pull request.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1143885848
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
Review Comment:
Unfortunately, yes. If the Docker build is busy for a while and doesn't download anything, the http pool times out apparently. There have been other projects that resolved this in a similar way like [pulsar](https://github.com/apache/pulsar/pull/8386).
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] gyogal commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "gyogal (via GitHub)" <gi...@apache.org>.
gyogal commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1498993782
Thanks a lot for working on this @dacort! I agree with @ksumit's points and if you feel like this is good to go as is, we could merge it to restore CI functionality for the project.
Post-merge would it be possible to run this for PRs received recently where the builds were not kicked off (such as #396)?
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on a diff in pull request #393: Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1135959794
##########
.github/workflows/build-ci-image.yaml:
##########
@@ -0,0 +1,46 @@
+#
+# 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 CI images'
+on:
+ push:
+ branches: ["main", "feature/github-actions"]
Review Comment:
Yes, I'll add a `path` on this that limits the action to if the `./dev/docker/Dockerfile` changes and also remove the feature branch - had that here so I could test the action.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on a diff in pull request #393: Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1135961100
##########
.github/workflows/unit-tests.yaml:
##########
@@ -0,0 +1,29 @@
+#
+# 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 test
+on: [push]
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ steps:
+ -
+ name: Checkout
+ uses: actions/checkout@v3
+ -
+ name: Build with Maven
+ run: mvn -Pthriftserver -DskipITs -Dmaven.javadoc.skip=true -B -V -e verify
Review Comment:
Yes, planning on adding the other pre-existing jobs from `.travis.yml` file, which includes thriftserver unit tests and Spark 3.x tests for each.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] codecov-commenter commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1470970692
## [Codecov](https://codecov.io/gh/apache/incubator-livy/pull/393?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#393](https://codecov.io/gh/apache/incubator-livy/pull/393?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (01b12d4) into [master](https://codecov.io/gh/apache/incubator-livy/commit/45e07fec68f2b9ad1dc7ebce8db08ad8a778dddc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (45e07fe) will **decrease** coverage by `2.78%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #393 +/- ##
============================================
- Coverage 68.37% 65.60% -2.78%
+ Complexity 849 822 -27
============================================
Files 103 103
Lines 5989 5989
Branches 911 911
============================================
- Hits 4095 3929 -166
- Misses 1330 1534 +204
+ Partials 564 526 -38
```
[see 24 files with indirect coverage changes](https://codecov.io/gh/apache/incubator-livy/pull/393/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1142789396
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ # TODO: Possibly point to the ./build-ci-image.yaml with the "uses" key
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ strategy:
+ matrix:
+ spark_version: ["2.4", "3.0"]
Review Comment:
I think we can add the unit-test vs integration-test logic here.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1149796150
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ # TODO: Possibly point to the ./build-ci-image.yaml with the "uses" key
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ strategy:
+ matrix:
+ spark_version: ["2.4", "3.0"]
Review Comment:
@dacort I wouldn't want to hold this PR for minor things. If you feel comfortable with the current state, I would push it as it is (something better than nothing) and then iterate on the improvements later on. Let me know what you think.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on a diff in pull request #393: Add GitHub Action for CI image
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1135896544
##########
.github/workflows/build-ci-image.yaml:
##########
@@ -0,0 +1,46 @@
+#
+# 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 CI images'
+on:
+ push:
+ branches: ["main", "feature/github-actions"]
Review Comment:
would this trigger a build every time there is a change in these branches? Is there a way to filter that only changes in this file will trigger this build?
##########
.github/workflows/unit-tests.yaml:
##########
@@ -0,0 +1,29 @@
+#
+# 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 test
+on: [push]
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ steps:
+ -
+ name: Checkout
+ uses: actions/checkout@v3
+ -
+ name: Build with Maven
+ run: mvn -Pthriftserver -DskipITs -Dmaven.javadoc.skip=true -B -V -e verify
Review Comment:
I'm assuming we will add more jobs that cover other scenarios?
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1500720773
Thanks @gyogal - Yea, @ksumit I think for now we should merge this. I imagine the PR workflow and unit-test workflow will diverge in the future so would rather get this merged and iterate.
@gyogal re: run the checks retroactively, I don't think it'll happen automatically but if folks push another commit it should.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1142783327
##########
.github/workflows/build-ci-image.yaml:
##########
@@ -0,0 +1,48 @@
+#
+# 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 CI images'
+on:
+ push:
+ branches: ["main"]
+ paths:
+ - 'dev/docker/Dockerfile'
+jobs:
+ docker-build:
+ runs-on: ubuntu-latest
+ steps:
+ -
+ name: Checkout
+ uses: actions/checkout@v3
+ -
+ name: Set up Docker Buildx
+ uses: docker/setup-buildx-action@v2
+ -
+ name: Login to the GitHub Container Registry
+ uses: docker/login-action@v2
+ with:
+ registry: ghcr.io
+ username: ${{ github.repository_owner }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ -
+ name: Build and push image
+ id: docker_build
+ uses: docker/build-push-action@v4
+ with:
+ push: true
Review Comment:
this is where we could change the logic to false if it's a PR build?
##########
.github/workflows/build-ci-image.yaml:
##########
@@ -0,0 +1,48 @@
+#
+# 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 CI images'
+on:
+ push:
+ branches: ["main"]
+ paths:
+ - 'dev/docker/Dockerfile'
Review Comment:
1. should we add this file in this list too? we would like to trigger a build for changes in this file as well right?
2. do we need a PR pipeline for validating PRs? I think all the logic will remain the same, we will build the image but won't push it unless it's a CI build? Should we use branches to determine a CI vs PR build, for example if the branch matches a pattern including "release-XXX"?
##########
.github/workflows/unit-tests.yaml:
##########
@@ -0,0 +1,52 @@
+#
+# 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: Unit Tests
+on: [push]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
Review Comment:
same as in `integration-tests.yaml`?
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ # TODO: Possibly point to the ./build-ci-image.yaml with the "uses" key
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ strategy:
+ matrix:
+ spark_version: ["2.4", "3.0"]
+ steps:
+ -
+ name: Checkout
+ uses: actions/checkout@v3
+ -
+ name: Cache local Maven repository
+ uses: actions/cache@v3
+ with:
+ path: |
+ /root/.m2/repository
+ !/root/.m2/repository/org/apache/livy
+ key: ${{ runner.os }}-maven-${{ hashFiles('pom.xml', '*/pom.xml', 'thriftserver/*/pom.xml', 'core/*/pom.xml', 'repl/*/pom.xml', 'scala-api/*/pom.xml') }}
Review Comment:
does `hashFiles` support `**/pom.xml`?
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
Review Comment:
do we need the parameters other than the retry count logic?
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1478473651
> Would it make sense to merge `integration-tests.yaml` and `unit-tests.yaml`
I wasn't quite sure if there was an easy way to do that. Can definitely look more into it, but ideas welcome.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1143898274
##########
.github/workflows/integration-tests.yaml:
##########
@@ -0,0 +1,54 @@
+#
+# 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: Integration Tests
+on:
+ pull_request:
+ types: [opened, reopened, synchronize]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
+jobs:
+ build:
+ runs-on: ubuntu-20.04
+ # TODO: Possibly point to the ./build-ci-image.yaml with the "uses" key
+ container: ghcr.io/${{ github.repository_owner }}/livy-ci:latest
+ strategy:
+ matrix:
+ spark_version: ["2.4", "3.0"]
+ steps:
+ -
+ name: Checkout
+ uses: actions/checkout@v3
+ -
+ name: Cache local Maven repository
+ uses: actions/cache@v3
+ with:
+ path: |
+ /root/.m2/repository
+ !/root/.m2/repository/org/apache/livy
+ key: ${{ runner.os }}-maven-${{ hashFiles('pom.xml', '*/pom.xml', 'thriftserver/*/pom.xml', 'core/*/pom.xml', 'repl/*/pom.xml', 'scala-api/*/pom.xml') }}
Review Comment:
It does, but due to the use of the custom `container` there are directories created during the build process that subsequent GitHub actions don't have the ability to access, receiving a "permission denied" error while attempting to process. There's some more info about it here: https://github.com/actions/runner/issues/449
There's a couple options:
- Change the userid that the container is running as to specifically be `1001`.
- Be more specific with the `pom.xml` paths
Option 2 seemed a little safer.
##########
.github/workflows/unit-tests.yaml:
##########
@@ -0,0 +1,52 @@
+#
+# 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: Unit Tests
+on: [push]
+env:
+ MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3
Review Comment:
Yep.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] gyogal commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "gyogal (via GitHub)" <gi...@apache.org>.
gyogal commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1503237784
Thank you for your work on this! I have merged your commit and pre-commit checks are looking good now. There are two minor issues I noticed (maybe these are just temporary and will go away after a few builds):
1. codecov seems to report an unrealistic drop in coverage on PRs (Merging ... will decrease coverage by 40.09%.)
2. post-commit unit tests are reporting failures in org.apache.livy.rsc.TestSparkClient
Again, many thanks for adding these workflows because it unblocks PRs and is of great help for the project!
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] gyogal merged pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "gyogal (via GitHub)" <gi...@apache.org>.
gyogal merged PR #393:
URL: https://github.com/apache/incubator-livy/pull/393
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1503466473
@gyogal Yep I noticed that too as I work on #392. I initially thought it was related to my changes, but then noticed the failures on the main branch after this PR got merged in. I'm not quite sure what's going on as the unit tests ran fine in the PR.
I'm also guessing the codecov might be related to the unit tests not running but that's just a theory at this point.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] ksumit commented on pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "ksumit (via GitHub)" <gi...@apache.org>.
ksumit commented on PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#issuecomment-1480416082
> > Would it make sense to merge `integration-tests.yaml` and `unit-tests.yaml`
>
> I wasn't quite sure if there was an easy way to do that. Can definitely look more into it, but ideas welcome.
Here is an example that i found on https://docs.github.com/en/actions/learn-github-actions/contexts
```
name: CI
on: push
jobs:
prod-check:
if: ${{ github.ref == 'refs/heads/main' }}
runs-on: ubuntu-latest
steps:
- run: echo "Deploying to production server on branch $GITHUB_REF"
```
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-livy] dacort commented on a diff in pull request #393: [LIVY-972] Add GitHub Action for CI image
Posted by "dacort (via GitHub)" <gi...@apache.org>.
dacort commented on code in PR #393:
URL: https://github.com/apache/incubator-livy/pull/393#discussion_r1143902325
##########
.github/workflows/build-ci-image.yaml:
##########
@@ -0,0 +1,48 @@
+#
+# 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 CI images'
+on:
+ push:
+ branches: ["main"]
+ paths:
+ - 'dev/docker/Dockerfile'
Review Comment:
1. Yes, good idea, will add that.
2. Hm, good point. I had the `main` branch here so we wouldn't push the image unless it got merged into main, but would be good to be able to validate the image as well before doing so.
--
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: commits-unsubscribe@livy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org