- Notifications
You must be signed in to change notification settings - Fork 11
add mac arm ci automation #122
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
WalkthroughAdds a new GitHub Actions workflow to run macOS ARM (Apple Silicon) tests with two jobs: a multi‑Python build/test job and a NumPy compatibility matrix test job targeting macOS ARM64 runners. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test-macos-arm.yaml (1)
31-40: Consider extracting Rust and toolchain setup to a reusable workflow.Both jobs install Rust identically (lines 31–34 and 84–87). This duplication can be reduced by extracting the common setup steps into a reusable workflow or a composite action, improving maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test-macos-arm.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: amunra Repo: questdb/py-questdb-client PR: 120 File: ci/run_tests_pipeline.yaml:85-85 Timestamp: 2025-11-28T17:46:34.703Z Learning: In the py-questdb-client repository, Python version testing is handled by cibuildwheel configuration in pyproject.toml. The ci/run_tests_pipeline.yaml pipeline is specifically for testing against various numpy versions, not for comprehensive Python version coverage. ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel windows_i686)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel windows_x86_64)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel macos_x64)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel linux_x64_cpython_musllinux)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel linux_x64_pypy)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel linux_x64_cpython_manylinux_x86_64)
- GitHub Check: questdb.py-questdb-client-release (cibuildwheel start_linux_arm64_agent_aws)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing on windows-msvc-2019)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing on mac)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing on linux-qdb-master)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing on linux-old-pandas)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing on linux)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing TestsAgainstVariousNumpyVersion2x)
- GitHub Check: questdb.py-questdb-client-tests (Building and testing TestsAgainstVariousNumpyVersion1x)
- GitHub Check: Test numpy compatibility (3.11, 2.0.0)
🔇 Additional comments (4)
.github/workflows/test-macos-arm.yaml (4)
51-51: Verify thatJAVA_HOME_17_ARM64is available on macOS-13-xlarge ARM64 runners.The environment variable
JAVA_HOME_17_ARM64may not be available on this runner. Please confirm it exists or consider using an alternative approach to set JAVA_HOME. You can check GitHub's documentation or test this in a workflow run to verify availability.
92-93: Verify theuv run --python pythonsyntax is correct.The command
uv run --python pythonmay not work as intended. The--pythonflag typically requires a specific path, version identifier, or interpreter name thatuvcan resolve. Verify that this selects the correct Python version or use an explicit path (e.g.,--python python3.10).
92-93: The numpy compatibility test only runs a single test class.The test command runs only
TestBufferProtocolVersionV2instead of the full test suite. If this is intentional for targeted numpy validation, this should be documented in a comment. If it's an oversight, consider running the full test suite with the numpy version matrix.
36-40: Inconsistent toolchain setup between jobs.The
testjob installs Cython and usespython3 proj.py test, whiletest-numpy-compatskips Cython and usesuv run. Clarify whether Cython is required for the numpy compatibility tests or if the different invocation methods serve different purposes.Also applies to: 89-93
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.