You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/28 17:24:33 UTC
[GitHub] [arrow-rs] alamb opened a new pull request, #2214: CI: Only run coverage jobs on master
alamb opened a new pull request, #2214:
URL: https://github.com/apache/arrow-rs/pull/2214
Draft until https://github.com/apache/arrow-rs/pull/2212
# Which issue does this PR close?
re https://github.com/apache/arrow-rs/issues/2149
# Rationale for this change
The codecov job is long ([example](https://github.com/apache/arrow-rs/runs/7564428465?check_suite_focus=true) takes 20 minutes, and I haven't seen its results referred to in PRs myself.
It also has several unfortunate issues (like not accounting for coverage of doc tests), which we should probably file as a separate issue if anyone is looking at its results.
Thus, let's only run it on pushes to master
# What changes are included in this PR?
1. Only run coverage job on pushes to master
# Are there any user-facing changes?
no
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199173198
Marking as a draft so we don't merge this PR accidentally
--
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-rs] ursabot commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1205669025
Benchmark runs are scheduled for baseline = d87f6a456330f0eff00541b38017e8a982bf8033 and contender = 5166a08600fa8b356808b3fecf484f6523020f7f. 5166a08600fa8b356808b3fecf484f6523020f7f is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/e081ac49a4ca4cb8aa81447005a85c4f...f5ca1ecea17a4b93997557d2e0fcb70a/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/26a15657472d412faf7f40297abde5d8...6e38a34e49354d50a5038c89ff8ec37e/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/294d226a0e694c9e998cc1cbf9b9a2ff...d26b339f44bf4747ab57da6e005253c9/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/1175ce0254e547fbb5f2fdad9ae42b09...2d253cdac5104aff8f19bbae60605596/)
Buildkite builds:
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199588176
For what it is worth I don't think this ticket is super high priority -- I was just going through all the CI checks in general (on the occasion of the `object_store` crate being merged and making the current intertwined ness just a bit too much to bear).
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199173003
> llvm-cov is generally faster than tarpaulin though, so I'll explore getting it to run without the action above
Thanks @nevi-me !
Note there is already one simple action in the `arrow-rs` repository https://github.com/apache/arrow-rs/tree/master/.github/actions/setup-builder in case you wanted to add another for coverage
--
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-rs] nevi-me commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
nevi-me commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199254910
Okay, thanks, I'll work on this on the weekend, if I can't make llvm-cov work, we can move to using tarpaulin only on master.
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199144796
> Won't this mean we lose information about test coverage in PRs?
Yes that is correct.
@nevi-me @HaoYang670 @jhorstmann @viirya @sunchao do you ever use the per PR code coverage report ? I am proposing to only run coverage jobs on master to improve our 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-rs] tustvold commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1204949593
Shall we update and get this one in, there seems to be fairly broad consensus?
--
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-rs] nevi-me commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
nevi-me commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199171031
On second thoughts, I believe we can't use certain actions, I wanted to add `taiki-e/install-action@cargo-llvm-cov`, but that might need INFRA approval.
llvm-cov is generally faster than tarpaulin though, so I'll explore getting it to run without the action above
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1205663479
> Shall we update and get this one in, there seems to be fairly broad consensus?
👍 I will do so. We'll save some CO2 and maybe help the other PRs to run a bit faster. We can always re-enable if someone wants to see it.
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1205666629
Well, this is promising -- the job is queued to run on the master commit:
https://github.com/apache/arrow-rs/commit/5166a08600fa8b356808b3fecf484f6523020f7f
https://github.com/apache/arrow-rs/runs/7679145290?check_suite_focus=true
--
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-rs] alamb merged pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb merged PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214
--
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-rs] alamb commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1205665749
I don't really know how to test this other than on master, so YOLO here we go:
--
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-rs] nevi-me commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
nevi-me commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199159735
> > Won't this mean we lose information about test coverage in PRs?
>
> Yes that is correct.
>
> @nevi-me @HaoYang670 @jhorstmann @viirya @sunchao do you ever use the per PR code coverage report ? I am proposing to only run coverage jobs on master to improve our CI
An alternative could be to run coverage as part of tests, via `cargo--llvm-cov`, which should also be faster. I'll make changes and PR them into this PR so we can discuss it.
I locally use `llvm-cov` so I do benefit from Rust coverage, but I'm inactive on arrow-rs at the moment, and would be fine if the decision was to disable coverage altogether if it isn't being used.
--
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-rs] viirya commented on pull request #2214: CI: Only run coverage jobs on master
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #2214:
URL: https://github.com/apache/arrow-rs/pull/2214#issuecomment-1199776096
I personally don't use it too much, so the idea of running coverage jobs on master is okay for me. But @nevi-me's proposal is awesome if it works. Thanks @nevi-me for working on it.
--
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