Fix update cargo dependencies script for Mac OS X #1010
Conversation
ayushdg
requested review from
andygrove,
charlesbluca,
galipremsagar and
jdye64
as code owners
There was a problem hiding this comment.
Thanks @ayushdg! LGTM
One thing this PR highlights is that we should add an option to use upstream DataFusion as part of the Python testing matrix, so that we could verify that this change fixes the issues on macOS; right now we only have this option for the Rust testing, which is limited to Ubuntu
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #1010 +/- ##
==========================================
+ Coverage 80.47% 80.59% +0.11%
==========================================
Files 75 75
Lines 4231 4231
Branches 765 765
==========================================
+ Hits 3405 3410 +5
+ Misses 662 653 -9
- Partials 164 168 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Good point. It should be fairly straightforward to add the same logic from the rust testing to our python test matrix as well. Happy to include in this pr |
Sounds good to me |
|
Link to a run with [test-df-upstream] enabled. |
|
@charlesbluca should be ready for another round of reviews |
| detect-ci-trigger: | ||
| name: Check for upstream trigger phrase | ||
| detect-upstream-trigger: | ||
| name: Check for upstream trigger phrase to test with upstream dask | ||
| runs-on: ubuntu-latest | ||
| if: github.repository == 'dask-contrib/dask-sql' | ||
| outputs: | ||
| triggered: ${{ steps.detect-trigger.outputs.trigger-found }} | ||
| triggered: ${{ steps.detect-upstream-trigger.outputs.trigger-found }} | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| with: | ||
| fetch-depth: 2 | ||
| - uses: xarray-contrib/ci-trigger@v1.2 | ||
| id: detect-trigger | ||
| id: detect-upstream-trigger | ||
| with: | ||
| keyword: "[test-upstream]" | ||
| detect-datafusion-upstream-trigger: | ||
| name: Check for upstream trigger phrase to test with upstream datafusion | ||
| runs-on: ubuntu-latest | ||
| if: github.repository == 'dask-contrib/dask-sql' | ||
| outputs: | ||
| triggered: ${{ steps.detect-datafusion-upstream-trigger.outputs.trigger-found }} | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| with: | ||
| fetch-depth: 2 | ||
| - uses: xarray-contrib/ci-trigger@v1.2 | ||
| id: detect-datafusion-upstream-trigger | ||
| with: | ||
| keyword: "[test-df-upstream]" |
There was a problem hiding this comment.
Since these are fairly quick steps, wonder if it would make sense to consolidate this into a single job with two steps?
The existing update-dependencies.sh script fails on Mac OS X as seen in these runs. This updates the script to use the
-Eas discussed in the comments here.