chore: Build and publish RAI packages via CI #731
Conversation
Juliaj
force-pushed
the
jj/ci/wheel_distro
branch
from
d60e381 to
db3948a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #731 +/- ##
=======================================
Coverage 65.34% 65.34%
=======================================
Files 78 78
Lines 3388 3388
=======================================
Hits 2214 2214
Misses 1174 1174 ☔ View full report in Codecov by Sentry. |
Juliaj
changed the title
Juliaj
force-pushed
the
jj/ci/wheel_distro
branch
from
cfc9b12 to
a9c8f68
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis PR introduces a comprehensive PyPI package publishing infrastructure including GitHub Actions workflows for discovering and publishing packages, Python helper scripts for package discovery and validation, documentation for the release process, a minimal test package, and corresponding test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Disc as discover_packages.py
participant Val as validate_packages.py
participant Pypi as PyPI Query
participant Build as Build Tools<br/>(cibuildwheel/<br/>Poetry)
participant TestPyPI as Test PyPI
participant PyPI as PyPI
GHA->>Disc: Discover packages<br/>from src/
Disc-->>GHA: Package metadata<br/>(name, version,<br/>has_c_extensions)
GHA->>Val: Validate requested<br/>packages
Val->>Pypi: Check version<br/>exists
Pypi-->>Val: Version status
Val-->>GHA: Validated packages
GHA->>Build: Build wheels &<br/>sdists
Build-->>GHA: Artifacts
GHA->>TestPyPI: Publish artifacts
TestPyPI-->>GHA: Published
GHA->>TestPyPI: Verify install
TestPyPI-->>GHA: Success
GHA->>PyPI: Publish artifacts
PyPI-->>GHA: Published
GHA->>PyPI: Verify install
PyPI-->>GHA: Success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/rai_extensions/rai_perception/pyproject.toml (1)
3-4: Consider removing the stale TODO comment.The TODO on line 3 states "update the version once it is published to PyPi" but the version has been updated. If this package is now ready for publishing, consider removing or updating this comment to avoid confusion.
src/rai_tiny/pyproject.toml (2)
5-10: Author format differs from other packages in the repository.Other packages use the format
"Name <email@domain.ai>"with organizational attribution (e.g.,"Jakub Matejczyk <jakub.matejczyk@robotec.ai>"). Consider aligning with this convention for consistency.🔎 Suggested format
-authors = ["Julia Jia"] +authors = ["Julia Jia <julia.jia@robotec.ai>"]
11-15: Consider whether "Beta" status is appropriate for a test package.The classifier
"Development Status :: 4 - Beta"implies a relatively mature package. Since this is explicitly a minimal test package for workflow validation, consider using"Development Status :: 3 - Alpha"or omitting the development status classifier entirely..github/workflows/pkg-publish.yaml (1)
173-209: Consider extracting artifact organization into a reusable action.The artifact organization logic is duplicated between the Test PyPI and PyPI publish jobs. While this duplication ensures independence, extracting this into a composite action or bash script would improve maintainability.
Also applies to: 252-288
scripts/pypi_query.py (2)
82-83: Consider using semantic version sorting.The current implementation uses lexicographic string sorting, which can produce unexpected results for version numbers (e.g., "10.0.0" would sort before "2.0.0"). For more accurate version ordering, consider using a semantic versioning library like
packaging.version.🔎 Example refactor using packaging library
+from packaging.version import parse as parse_version + def get_pypi_versions(package_name: str, test_pypi: bool = False) -> list[str]: """Get all versions of a package from PyPI or Test PyPI. Args: package_name: Name of the package test_pypi: If True, query Test PyPI instead of PyPI Returns: List of versions sorted newest first """ data = _fetch_pypi_data(package_name, test_pypi) if data is None: return [] releases = data.get("releases", {}) - return sorted(releases.keys(), reverse=True) + try: + return sorted(releases.keys(), key=parse_version, reverse=True) + except Exception: + # Fall back to string sorting if version parsing fails + return sorted(releases.keys(), reverse=True)
113-124: Clarify the inverted return code convention.The
checkcommand returns 1 when the version exists and 0 when it doesn't exist, which is inverted from typical Unix conventions (where 0 indicates success). While this appears intentional for the publishing workflow (where an existing version is an error condition), it could be confusing for CLI users who expect0 = success.Consider either:
- Documenting this behavior clearly in the help text
- Aligning with Unix conventions and adjusting the calling code in
validate_packages.py
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.github/workflows/list-packages.yaml.github/workflows/pkg-publish.yamlCONTRIBUTING.mddocs/pypi_release.mdscripts/discover_packages.pyscripts/pypi_query.pyscripts/validate_packages.pysrc/rai_bench/pyproject.tomlsrc/rai_extensions/rai_perception/pyproject.tomlsrc/rai_tiny/README.mdsrc/rai_tiny/pyproject.tomlsrc/rai_tiny/rai_tiny/__init__.pytests/pkg_publish/__init__.pytests/pkg_publish/test_discover_packages.pytests/pkg_publish/test_pypi_query.pytests/pkg_publish/test_validate_packages.pytests/smoke/import_test.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-01-25T14:33:56.413Z
Learnt from: maciejmajek
Repo: RobotecAI/rai PR: 386
File: src/rai/rai/tools/ros/tools.py:30-30
Timestamp: 2025-01-25T14:33:56.413Z
Learning: In the RAI package, ROS2-related Python packages (e.g., tf_transformations) are currently installed via rosdep rather than being declared in pyproject.toml.
Applied to files:
src/rai_tiny/README.mdsrc/rai_extensions/rai_perception/pyproject.tomlsrc/rai_tiny/pyproject.toml
📚 Learning: 2024-12-02T13:52:07.099Z
Learnt from: boczekbartek
Repo: RobotecAI/rai PR: 314
File: src/rai_bringup/launch/sim_whoami_demo.launch.py:0-0
Timestamp: 2024-12-02T13:52:07.099Z
Learning: The `rai_examples` package does not exist yet, so references to it using `FindPackageShare` will not work.
Applied to files:
src/rai_tiny/README.md
📚 Learning: 2024-11-28T20:32:28.114Z
Learnt from: maciejmajek
Repo: RobotecAI/rai PR: 327
File: tests/core/test_tool_runner.py:48-63
Timestamp: 2024-11-28T20:32:28.114Z
Learning: In tests for the `ToolRunner`, since commands are validated by pydantic using `Literal` types, testing with invalid commands would primarily test pydantic validation rather than the `ToolRunner`. Therefore, testing with an invalid tool name is appropriate to assess the `ToolRunner`'s error handling.
Applied to files:
tests/pkg_publish/test_validate_packages.py
📚 Learning: 2024-11-28T14:35:16.591Z
Learnt from: boczekbartek
Repo: RobotecAI/rai PR: 324
File: src/rai_whoami/setup.py:31-34
Timestamp: 2024-11-28T14:35:16.591Z
Learning: In the `rai_whoami` package, only `.py` launch files are present, so the launch file glob pattern in `setup.py` does not need to include `.xml` files.
Applied to files:
tests/smoke/import_test.py
🧬 Code graph analysis (3)
scripts/validate_packages.py (1)
scripts/pypi_query.py (1)
main(86-141)
tests/pkg_publish/test_pypi_query.py (1)
scripts/pypi_query.py (3)
check_version(51-66)get_pypi_versions(69-83)main(86-141)
scripts/pypi_query.py (1)
scripts/discover_packages.py (1)
main(109-128)
🪛 LanguageTool
docs/pypi_release.md
[uncategorized] ~92-~92: The official name of this software platform is spelled with a capital “H”.
Context: ...s: check, list) The workflow YAML (.github/workflows/pkg-publish.yaml) orchestrat...
(GITHUB)
[grammar] ~150-~150: Use a hyphen to join words.
Context: ...hown but don't block publishing Wheel building issues: - Pure Python pack...
(QB_NEW_EN_HYPHEN)
🪛 Ruff (0.14.10)
scripts/discover_packages.py
65-66: try-except-pass detected, consider logging the exception
(S110)
65-65: Do not catch blind exception: Exception
(BLE001)
87-88: try-except-continue detected, consider logging the exception
(S112)
87-87: Do not catch blind exception: Exception
(BLE001)
scripts/validate_packages.py
126-126: subprocess call: check for execution of untrusted input
(S603)
127-127: Starting a process with a partial executable path
(S607)
141-141: subprocess call: check for execution of untrusted input
(S603)
142-149: Starting a process with a partial executable path
(S607)
tests/pkg_publish/test_validate_packages.py
78-78: Unused method argument: capsys
(ARG002)
226-226: Unused method argument: capsys
(ARG002)
scripts/pypi_query.py
40-40: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
🔇 Additional comments (24)
src/rai_extensions/rai_perception/pyproject.toml (1)
12-18: Dependencies and Python constraint look appropriate.The Python version constraint
^3.10, <3.13aligns with other packages in this PR, and the explicit dependencies onrai-gsam2andrai_coreare properly versioned.src/rai_tiny/README.md (1)
1-5: LGTM!Clear and concise README that appropriately documents the purpose of this test package.
CONTRIBUTING.md (1)
62-82: LGTM!Clear documentation of PyPI naming conventions with practical examples. The distinction between PyPI package names (hyphens) and Python module names (underscores) is well explained and aligns with Python packaging best practices.
tests/smoke/import_test.py (1)
25-26: LGTM!Adding
rai_tinyto the ignore list is appropriate since it's a minimal test package for workflow validation rather than a production package requiring import validation.src/rai_bench/pyproject.toml (1)
11-12: LGTM!The Python version constraint
^3.10, <3.13is consistent with other packages in this PR and provides a reasonable upper bound for compatibility.src/rai_tiny/pyproject.toml (1)
20-24: LGTM!The Python version constraint is consistent with other packages, and the
[tool.rai]section withhas_c_extensions = falseprovides useful metadata for the publishing workflow.src/rai_tiny/rai_tiny/__init__.py (1)
1-25: LGTM! Clean minimal test package.The implementation serves its purpose as a lightweight package for testing the publishing workflow. The code is simple, correct, and appropriately scoped.
scripts/discover_packages.py (2)
56-74: LGTM! C-extension detection logic is sound.The detection strategy correctly prioritizes explicit markers, checks setup.py content, and scans for C/C++/Cython source files. The broad exception handling on lines 62-66 is appropriate here since this is a discovery script that should continue even when individual files are unreadable.
76-106: LGTM! Package discovery logic is robust.The function correctly handles missing directories, parses pyproject.toml files, and integrates C-extension detection. The broad exception handling on lines 86-88 appropriately allows the script to continue discovering other packages when one fails to parse.
tests/pkg_publish/test_discover_packages.py (1)
1-239: Excellent test coverage!The test suite comprehensively covers parsing, detection logic, edge cases, and CLI behavior. The use of parametrization and fixtures is appropriate, and the tests validate both success paths and error conditions.
tests/pkg_publish/test_pypi_query.py (1)
1-181: LGTM! Comprehensive test coverage for PyPI querying.The tests properly mock external API calls, validate both PyPI and Test PyPI paths, and cover CLI interactions including edge cases like missing packages and JSON output formatting.
tests/pkg_publish/test_validate_packages.py (1)
1-462: Excellent comprehensive test suite!The tests thoroughly validate package name extraction, C-extension checks, variant matching logic (hyphen/underscore), PyPI version validation, and CLI behavior. The test cases appropriately cover edge cases like exact-match priority when duplicate variants exist and proper error signaling.
.github/workflows/pkg-publish.yaml (2)
38-115: Well-structured wheel building job.The job correctly handles both pure Python and C-extension packages with appropriate build strategies. The Rust toolchain setup and Poetry integration are properly configured for packages with native dependencies.
102-102: No action needed—Python constraint is intentional and project-wide.The
<3.13constraint is not arbitrary to the CI workflow. It is consistently declared across all projectpyproject.tomlfiles (root, rai_bench, rai_core, rai_extensions/rai_perception), and rai_finetune is even more restrictive with<3.12. Comments in both the workflow ("once we move to python 3.11+, we can upgrade cibuildwheel") and in rai_finetune ("Python3.10 is recommended for better compatibility with other Rai components") document that this is a deliberate constraint based on dependency compatibility requirements, not an arbitrary cap.Likely an incorrect or invalid review comment.
scripts/pypi_query.py (4)
1-23: LGTM! Clean imports and module setup.The copyright header, module docstring, and imports are well-structured and appropriate for the script's purpose.
26-48: LGTM! Error handling is robust.The function correctly handles all expected error cases (404, network errors, timeouts, JSON parsing). The static analysis warning (S310) about URL schemes is a false positive since the URLs are constructed internally with known safe HTTPS endpoints.
51-66: LGTM! Clear and correct logic.The function appropriately checks for version existence in the releases dictionary.
144-145: LGTM! Standard script entry point.The
__main__block correctly invokes the main function with command-line arguments.scripts/validate_packages.py (6)
26-53: LGTM! Well-designed variant matching logic.The function correctly handles both exact matches (preferred) and variant matches (hyphen/underscore swapping), returning a tuple that indicates which type of match was found. This is useful for providing informative feedback to users.
76-110: LGTM! Clean package validation logic.The validation flow correctly loads packages, resolves names with variant matching, and provides helpful user feedback. The error handling appropriately lists available packages when validation fails.
161-168: LGTM! Proper output handling.The function correctly outputs validated packages as JSON and writes to the GitHub Actions output file when provided.
171-181: LGTM! Simple and effective helper.The function cleanly extracts and formats package names for shell usage.
184-203: LGTM! Correctly implements C extension detection.The function properly checks for C extensions and writes to GitHub Actions output in the expected format.
206-263: LGTM! Well-structured CLI with backward compatibility.The main function properly handles multiple commands with clear usage messages and maintains backward compatibility for the old calling convention. The environment variable fallback for
check-c-extis appropriate for GitHub Actions integration.
| workflow_dispatch: | ||
| schedule: | ||
| # Run every Monday at 9:00 AM UTC | ||
| - cron: '0 9 * * 1' |
There was a problem hiding this comment.
@maciejmajek, let me know whether this is necessary. I don't mind taking this out.
Purpose
Automate building and publishing of RAI packages to PyPI through GitHub Actions workflows. This includes building wheels and source distributions for all packages, publishing to Test PyPI for validation.
Proposed Changes
Testing
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.