You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "raulcd (via GitHub)" <gi...@apache.org> on 2023/03/09 10:13:10 UTC
[GitHub] [arrow] raulcd opened a new pull request, #34512: MINOR: [CI] Self-hosted ARM workflows improvements
raulcd opened a new pull request, #34512:
URL: https://github.com/apache/arrow/pull/34512
### Rationale for this change
Fixes some post-merge review comments
### What changes are included in this PR?
Comments from original: https://github.com/apache/arrow/pull/34482
### Are these changes tested?
On CI
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1134579066
##########
.github/workflows/cpp.yml:
##########
@@ -55,22 +55,39 @@ env:
jobs:
docker:
name: ${{ matrix.title }}
- runs-on: ubuntu-latest
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 75
strategy:
fail-fast: false
matrix:
- image:
- - conda-cpp
- - ubuntu-cpp-sanitizer
include:
- - image: conda-cpp
+ - arch: amd64
+ clang_tools: "14"
Review Comment:
Could you use `-` not `_` for word separator for `matrix` parameter names like `runs-on`?
```suggestion
clang-tools: "14"
```
##########
.github/workflows/cpp.yml:
##########
@@ -55,22 +55,39 @@ env:
jobs:
docker:
name: ${{ matrix.title }}
- runs-on: ubuntu-latest
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 75
strategy:
fail-fast: false
matrix:
- image:
- - conda-cpp
- - ubuntu-cpp-sanitizer
include:
- - image: conda-cpp
+ - arch: amd64
+ clang_tools: "14"
Review Comment:
Because it's GitHub Actions' convention.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #34512: MINOR: [CI] Self-hosted ARM workflows improvements
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1132827090
##########
.github/workflows/go.yml:
##########
@@ -42,20 +42,31 @@ permissions:
jobs:
docker:
- name: AMD64 Debian 11 Go ${{ matrix.go }}
- runs-on: ubuntu-latest
+ name: ${{ matrix.arch-label }} Debian 11 Go ${{ matrix.go }}
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 60
strategy:
fail-fast: false
matrix:
- go: [1.17, 1.18]
include:
- - go: 1.17
+ - arch-label: AMD64
+ arch: amd64
+ go: 1.17
+ runs-on: ubuntu-latest
staticcheck: v0.2.2
- - go: 1.18
+ - arch-label: AMD64
+ arch: amd64
+ go: 1.18
+ runs-on: ubuntu-latest
+ staticcheck: v0.3.3
+ - arch-label: ARM64
+ arch: arm64v8
+ go: 1.17
staticcheck: v0.3.3
Review Comment:
Could you use `v0.2.2` for Go 1.17 like the existing 1.17 job?
```suggestion
staticcheck: v0.2.2
```
Could you also remove needless code?
```diff
diff --git a/ci/scripts/go_build.sh b/ci/scripts/go_build.sh
index c113bbd320..3c8cc0f4ee 100755
--- a/ci/scripts/go_build.sh
+++ b/ci/scripts/go_build.sh
@@ -20,13 +20,6 @@
set -ex
source_dir=${1}/go
-ARCH=`uname -m`
-
-# Arm64 CI is triggered by travis and run in arm64v8/golang:1.17-bullseye
-if [ "aarch64" == "$ARCH" ]; then
-# Install `staticcheck`
- GO111MODULE=on go install honnef.co/go/tools/cmd/staticcheck@v0.2.2
-fi
pushd ${source_dir}/arrow
```
##########
.github/workflows/cpp.yml:
##########
@@ -401,15 +401,14 @@ jobs:
PATH="$(cygpath --unix ${PYTHON_BIN_DIR}):${PATH}"
ci/scripts/cpp_test.sh "$(pwd)" "$(pwd)/build"
- linux-arm:
+ docker-arm:
Review Comment:
Can we unify this to the existing `docker` job like Go's one?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34512:
URL: https://github.com/apache/arrow/pull/34512#issuecomment-1468523070
Benchmark runs are scheduled for baseline = 591042fcdd744c2e4cc0cec3d3c0c7fc1baf0ca5 and contender = 0582122b41949790b795c3760ab41f21275f393c. 0582122b41949790b795c3760ab41f21275f393c is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/23ba0182c3ab4c63b2953b4aa45b4ad8...b36ac80d86ba4b3eba7ee8a9a3b1f455/)
[Finished :arrow_down:0.3% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/2237f04a1a354075bc334563e17071ca...41116055d4de4e17915acfb5a1e9b8c9/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/88fb8aea5e6f48b2b5369a40edf8cb7b...c2b535ec182741d8a009ba6ce0506f50/)
[Finished :arrow_down:0.22% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/4bd656a799cd4b71b5fa0ff9159c5e04...42298ce99c30489d92c4e9ee705df5b5/)
Buildkite builds:
[Finished] [`0582122b` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2521)
[Finished] [`0582122b` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2551)
[Finished] [`0582122b` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2519)
[Finished] [`0582122b` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2542)
[Finished] [`591042fc` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2520)
[Finished] [`591042fc` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2550)
[Finished] [`591042fc` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2518)
[Finished] [`591042fc` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2541)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] raulcd commented on pull request #34512: MINOR: [CI] Self-hosted ARM workflows improvements
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on PR #34512:
URL: https://github.com/apache/arrow/pull/34512#issuecomment-1463648856
The job failure is not related to the PR
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34512:
URL: https://github.com/apache/arrow/pull/34512#issuecomment-1465984595
* Closes: #34543
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou merged pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #34512:
URL: https://github.com/apache/arrow/pull/34512
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1134579066
##########
.github/workflows/cpp.yml:
##########
@@ -55,22 +55,39 @@ env:
jobs:
docker:
name: ${{ matrix.title }}
- runs-on: ubuntu-latest
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 75
strategy:
fail-fast: false
matrix:
- image:
- - conda-cpp
- - ubuntu-cpp-sanitizer
include:
- - image: conda-cpp
+ - arch: amd64
+ clang_tools: "14"
Review Comment:
Could you use `-` not `_` for word separator for `matrix` parameter names like `ruins-on`?
```suggestion
clang-tools: "14"
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] raulcd commented on a diff in pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1135111860
##########
.github/workflows/cpp.yml:
##########
@@ -55,22 +55,39 @@ env:
jobs:
docker:
name: ${{ matrix.title }}
- runs-on: ubuntu-latest
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 75
strategy:
fail-fast: false
matrix:
- image:
- - conda-cpp
- - ubuntu-cpp-sanitizer
include:
- - image: conda-cpp
+ - arch: amd64
+ clang_tools: "14"
Review Comment:
sure, I've done it. Just so I know in the future, is there a reason for it I am not aware of?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] raulcd commented on a diff in pull request #34512: GH-34543: [CI] Self-hosted ARM workflows improvements
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1135126778
##########
.github/workflows/cpp.yml:
##########
@@ -55,22 +55,39 @@ env:
jobs:
docker:
name: ${{ matrix.title }}
- runs-on: ubuntu-latest
+ runs-on: ${{ matrix.runs-on }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 75
strategy:
fail-fast: false
matrix:
- image:
- - conda-cpp
- - ubuntu-cpp-sanitizer
include:
- - image: conda-cpp
+ - arch: amd64
+ clang_tools: "14"
Review Comment:
ok, that makes sense. I found some `_` sometimes but in general they seem to use `-`. There is even a question opened on if the convention exists: https://github.com/community/community/discussions/39547
I'll have it in mind in the future so at least we have a convention. Thanks @kou
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org