omniverseclient pin#5487
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR pins the omniverseclient dependency from an unpinned version to 2.71.1.7015 in both source/isaaclab/setup.py and tools/wheel_builder/res/python_packages.toml. This is a straightforward dependency version lock to ensure reproducible builds and prevent unexpected breakage from upstream updates.
Architecture Impact
Self-contained. This change only affects dependency resolution at install time. No runtime code paths are modified. The pinned version will be resolved during pip install isaaclab and wheel building processes.
Implementation Verdict
Minor fixes needed — The core change is correct, but there's an inconsistency between the two files that should be addressed.
Test Coverage
Dependency pinning changes don't require unit tests. The appropriate verification is CI installation tests, which will validate that the pinned version resolves correctly. No test changes needed.
CI Status
No CI checks available yet. CI should be monitored to confirm the pinned version installs correctly across all supported platforms.
Findings
🟡 Warning: source/isaaclab/setup.py:27 vs tools/wheel_builder/res/python_packages.toml:15 — Inconsistent gymnasium version between files
The setup.py specifies gymnasium==1.2.1 while python_packages.toml specifies gymnasium==1.2.0. This pre-existing inconsistency should be noted — while not introduced by this PR, it's worth flagging since you're already touching these files for consistency purposes.
🔵 Improvement: PR Description — Missing changelog entry per contribution guidelines
The PR checklist indicates changelog updates should be made for version changes. Dependency pins that fix compatibility issues typically warrant a changelog note (e.g., "Pinned omniverseclient to 2.71.1.7015 for stability"). Consider whether this pin was made in response to a specific issue that should be documented.
🔵 Improvement: Both files — Consider adding a comment explaining why this version is pinned
Following the pattern used for other pins (e.g., line 36 in setup.py: "pillow==12.1.1" with comment # make sure this is consistent with isaac sim version), adding a brief comment explaining the reason for this specific pin would help future maintainers understand if/when it can be updated. Example: "omniverseclient==2.71.1.7015", # pinned for livestream compatibility with Isaac Sim 5.1
Greptile SummaryThis PR pins Confidence Score: 5/5Safe to merge — minimal, consistent change pinning a single dependency across two files. No logic changes; both dependency files are updated identically; changelog entry is present. No issues found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR: omniverseclient pin] --> B[source/isaaclab/setup.py]
A --> C[tools/wheel_builder/res/python_packages.toml]
A --> D[changelog.d/omniverseclient-pin.rst]
B -- "omniverseclient==2.71.1.7015" --> E[Runtime dependency pinned]
C -- "omniverseclient==2.71.1.7015" --> E
D -- "Documents the fix" --> F[Changelog updated]
Reviews (2): Last reviewed commit: "changelog fragment" | Re-trigger Greptile |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
The new commit adds a changelog entry (source/isaaclab/changelog.d/omniverseclient-pin.rst) addressing the previous feedback about missing documentation. The core dependency pin remains correct and consistent across both files.
Follow-up Assessment
Previous concern about missing changelog entry has been addressed. The new changelog fragment properly documents the pin under "Fixed".
The other previous findings (gymnasium version inconsistency between files, and suggestion for inline comments) were pre-existing issues and optional improvements respectively — neither blocks this PR.
Implementation Verdict
Ship it — The dependency pin is correctly applied in both locations, and the changelog entry has been added.
CI Status
Core checks passing (pre-commit ✅, Build Wheel ✅, changelog fragments ✅). Installation tests still pending but wheel build success indicates the pinned version resolves correctly.
Description
Update omniverseclient pin to
2.71.1.7015.Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there