-
Notifications
You must be signed in to change notification settings - Fork 251
Update: bump sail version #763
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the Sail benchmark scripts by bumping the pysail library version and updating related dependencies to ensure compatibility with newer versions.
Changes:
- Updated pysail from version >=0.4.2,<0.5.0 to >=0.4.6,<0.6.0
- Updated pyspark-client from version 4.0.0 to 4.1.1
- Removed explicit version constraints for protobuf, grpcio, and grpcio-status dependencies
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| sail/benchmark.sh | Updated dependency versions for the non-partitioned Sail benchmark setup |
| sail-partitioned/benchmark.sh | Updated dependency versions for the partitioned Sail benchmark setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "protobuf==5.28.3" \ | ||
| "grpcio==1.71.2" \ | ||
| "grpcio-status==1.71.2" \ | ||
| pip install --no-cache-dir "pysail>=0.4.6,<0.6.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions
-
Can the script please fix the
pysailversion? Right now, it installs some unspecified version between 0.4.6 and 0.6. As far as I see, the latest version in 0.4.6. With the current approach, future benchmark runs may yield different results than today's benchmark runs. -
Looking quickly at the sail changelog, I couldn't find any changes related to performance compared to the previous update to 0.4.2. Should we wait with a ClickBench PR until the next major release (0.5.x)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- My intention was to allow ClickBench to always use the latest patch version. Sail follows semantic versioning (
MAJOR.MINOR.PATCH) with frequent patch releases roughly every two weeks, while minor releases happen about every two months, with0.5expected next week. Rather than opening a new ClickBench PR for every patch release, I thought it would be more practical to update ClickBench only on minor releases. - Yeah forsure, happy to wait for the next minor release. Since
0.5is coming next week, that was the motivation for preparing this PR 🙂
No description provided.