Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Add build step to run-cluster #3606

Merged
merged 13 commits into from
Dec 19, 2024
Merged

ci: Add build step to run-cluster #3606

merged 13 commits into from
Dec 19, 2024

Conversation

raunakab
Copy link
Contributor

Overview

The run-cluster workflow now also builds the commit by calling the build-commit workflow first.

@github-actions github-actions bot added the ci label Dec 19, 2024
Copy link

codspeed-hq bot commented Dec 19, 2024

CodSpeed Performance Report

Merging #3606 will degrade performances by 30.61%

Comparing run-cluster-also-builds (8235760) with main (a76f800)

Summary

⚡ 2 improvements
❌ 1 regressions
✅ 24 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main run-cluster-also-builds Change
test_count[1 Small File] 4 ms 3.6 ms +11.8%
test_iter_rows_first_row[100 Small Files] 223.1 ms 197.8 ms +12.79%
test_show[100 Small Files] 15.8 ms 22.8 ms -30.61%

@raunakab
Copy link
Contributor Author

Example of a successful run:
https://github.com/Eventual-Inc/Daft/actions/runs/12403736836

@raunakab raunakab marked this pull request as ready for review December 19, 2024 00:40
@raunakab raunakab requested a review from jaychia December 19, 2024 00:40
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.80%. Comparing base (ca4d3f7) to head (8235760).
Report is 10 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3606   +/-   ##
=======================================
  Coverage   77.80%   77.80%           
=======================================
  Files         718      718           
  Lines       88176    88247   +71     
=======================================
+ Hits        68607    68664   +57     
- Misses      19569    19583   +14     

see 20 files with indirect coverage changes

- if daft-version is specified, then build-commit is skipped and the
  empty string is used instead of `needs.build.outputs.wheel_url`
    - remember, the `wheel_url` will never be set due to the fact that
      the `build` job was never run
- if the daft-version is *not* specified, then the `build` step *is* run
  and the `wheel_url` will properly be set
@raunakab
Copy link
Contributor Author

raunakab commented Dec 19, 2024

Example of a run that does not specify daft-version or daft-wheel-url:
https://github.com/Eventual-Inc/Daft/actions/runs/12409106683

Example of a run that specifies only daft-version:
https://github.com/Eventual-Inc/Daft/actions/runs/12409786569

Example of a run that specifies only daft-wheel-url:
https://github.com/Eventual-Inc/Daft/actions/runs/12409120134

@raunakab raunakab requested a review from jaychia December 19, 2024 08:15
.github/workflows/run-cluster.yaml Show resolved Hide resolved
.github/workflows/run-cluster.yaml Outdated Show resolved Hide resolved
@raunakab raunakab merged commit ea8f8bd into main Dec 19, 2024
48 of 51 checks passed
@raunakab raunakab deleted the run-cluster-also-builds branch December 19, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants