[wip] ci/cd: add nightly build and CI for flashinfer-python,flashinfer-jit-cache,flashinfer-cubin
#1867
Conversation
yzh119
changed the title
flashinfer-python,flashinfer-jit-cache,`flashinfer-cubinflashinfer-python,flashinfer-jit-cache,flashinfer-cubin
Summary of ChangesHello @yzh119, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the CI/CD pipeline for FlashInfer's Python packages, focusing on establishing a more robust and traceable build process for nightly and continuous integration. Key changes include embedding git commit information into package metadata, enabling flexible versioning for development releases, and overhauling the wheel index generation to support a multi-package, CUDA-aware structure. These enhancements aim to streamline the release process and improve the reliability of package distribution. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces changes to support nightly builds and enhance CI/CD processes by embedding git commit information into the packages. The changes are spread across several package build scripts and a new script to update the wheel index. My review focuses on improving the maintainability and correctness of this new build logic. I've identified an opportunity to reduce code duplication in the git version retrieval logic and a bug in the version parsing regex within the new update_whl_index.py script. Addressing these points will make the new CI/CD tooling more robust and easier to maintain.
| def get_package_info(wheel_path: pathlib.Path) -> Optional[dict]: | ||
| """Extract package information from wheel filename.""" | ||
| wheel_name = wheel_path.name | ||
|
|
||
| # Try flashinfer-python pattern | ||
| match = re.match(r"flashinfer_python-([0-9.]+(?:\.dev\d+)?)-", wheel_name) | ||
| if match: | ||
| version = match.group(1) | ||
| return { | ||
| "package": "flashinfer-python", | ||
| "version": version, | ||
| "cuda": None, | ||
| } | ||
|
|
||
| # Try flashinfer-cubin pattern | ||
| match = re.match(r"flashinfer_cubin-([0-9.]+(?:\.dev\d+)?)-", wheel_name) | ||
| if match: | ||
| version = match.group(1) | ||
| return { | ||
| "package": "flashinfer-cubin", | ||
| "version": version, | ||
| "cuda": None, | ||
| } | ||
|
|
||
| # Try flashinfer-jit-cache pattern (has CUDA suffix in version) | ||
| match = re.match(r"flashinfer_jit_cache-([0-9.]+(?:\.dev\d+)?\+cu\d+)-", wheel_name) | ||
| if match: | ||
| version = match.group(1) | ||
| cuda_ver = get_cuda_version(wheel_name) | ||
| return { | ||
| "package": "flashinfer-jit-cache", | ||
| "version": version, | ||
| "cuda": cuda_ver, | ||
| } | ||
|
|
||
| return None |
There was a problem hiding this comment.
The regular expressions used to extract version numbers are flawed. The pattern [0-9.]+ is too greedy and will incorrectly parse versions that include a .dev suffix (e.g., 0.3.1.dev123), leading to an incorrect version string like 0.3.1..dev123. This will cause issues when trying to process the wheel information.
I've provided a more robust implementation of get_package_info that uses a correct regex pattern for parsing PEP 440-style versions.
def get_package_info(wheel_path: pathlib.Path) -> Optional[dict]:
"""Extract package information from wheel filename."""
wheel_name = wheel_path.name
# A more robust version pattern, e.g., for 0.3.1 or 0.3.1.dev123
version_pattern = r"(\d+(?:\.\d+)*(?:\.dev\d+)?)"
# Try flashinfer-python pattern
match = re.match(rf"flashinfer_python-{version_pattern}-", wheel_name)
if match:
version = match.group(1)
return {
"package": "flashinfer-python",
"version": version,
"cuda": None,
}
# Try flashinfer-cubin pattern
match = re.match(rf"flashinfer_cubin-{version_pattern}-", wheel_name)
if match:
version = match.group(1)
return {
"package": "flashinfer-cubin",
"version": version,
"cuda": None,
}
# Try flashinfer-jit-cache pattern (has CUDA suffix in version)
jit_cache_version_pattern = r"(\d+(?:\.\d+)*(?:\.dev\d+)?\+cu\d+)"
match = re.match(rf"flashinfer_jit_cache-{jit_cache_version_pattern}-", wheel_name)
if match:
version = match.group(1)
cuda_ver = get_cuda_version(wheel_name)
return {
"package": "flashinfer-jit-cache",
"version": version,
"cuda": cuda_ver,
}
return None
flashinfer-cubin/build_backend.py
Outdated
| def _get_git_version(): | ||
| """Get git commit hash.""" | ||
| import subprocess | ||
|
|
||
| try: | ||
| git_version = ( | ||
| subprocess.check_output( | ||
| ["git", "rev-parse", "HEAD"], | ||
| cwd=Path(__file__).parent.parent, | ||
| stderr=subprocess.DEVNULL, | ||
| ) | ||
| .decode("ascii") | ||
| .strip() | ||
| ) | ||
| return git_version | ||
| except Exception: | ||
| return "unknown" |
There was a problem hiding this comment.
This _get_git_version function is also present in flashinfer-jit-cache/setup.py and the root setup.py. To improve maintainability and avoid code duplication, consider moving this function to a shared build utility module and importing it where needed.
Additionally, the except Exception: is too broad. It's better practice to catch more specific exceptions that you expect to handle. In this case, subprocess.CalledProcessError (if the git command fails) and FileNotFoundError (if git is not installed) are the most likely exceptions.
| def _get_git_version(): | |
| """Get git commit hash.""" | |
| import subprocess | |
| try: | |
| git_version = ( | |
| subprocess.check_output( | |
| ["git", "rev-parse", "HEAD"], | |
| cwd=Path(__file__).parent.parent, | |
| stderr=subprocess.DEVNULL, | |
| ) | |
| .decode("ascii") | |
| .strip() | |
| ) | |
| return git_version | |
| except Exception: | |
| return "unknown" | |
| def _get_git_version(): | |
| """Get git commit hash.""" | |
| import subprocess | |
| try: | |
| git_version = ( | |
| subprocess.check_output( | |
| ["git", "rev-parse", "HEAD"], | |
| cwd=Path(__file__).parent.parent, | |
| stderr=subprocess.DEVNULL, | |
| ) | |
| .decode("ascii") | |
| .strip() | |
| ) | |
| return git_version | |
| except (subprocess.CalledProcessError, FileNotFoundError): | |
| return "unknown" |
|
Moved to #1872 |
…jit-cache`, `flashinfer-cubin` (#1872) <!-- .github/pull_request_template.md --> ## 📌 Description Duplicate of #1867, created from flashinfer/nightly to get write permission. ## 🔍 Related Issues <!-- Link any related issues here --> ## 🚀 Pull Request Checklist Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete. ### ✅ Pre-commit Checks - [x] I have installed `pre-commit` by running `pip install pre-commit` (or used your preferred method). - [x] I have installed the hooks with `pre-commit install`. - [x] I have run the hooks manually with `pre-commit run --all-files` and fixed any reported issues. > If you are unsure about how to set up `pre-commit`, see [the pre-commit documentation](https://pre-commit.com/). ## 🧪 Tests - [x] Tests have been added or updated as needed. - [x] All tests are passing (`unittest`, etc.). ## Reviewer Notes <!-- Optional: anything you'd like reviewers to focus on, concerns, etc. -->
📌 Description
🔍 Related Issues
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes