Skip to content

Conversation

@kellyguo11
Copy link
Contributor

Description

Updates dependency and installation to pull omniverseclient from pypi.nvidia.com.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 17, 2025

Greptile Summary

Migrated omniverseclient installation from custom packman script to standard PyPI distribution via pypi.nvidia.com

Key changes:

  • Added PIP_EXTRA_INDEX_URL environment variable in isaaclab.sh to enable pip to find packages on pypi.nvidia.com
  • Added omniverseclient to INSTALL_REQUIRES in source/isaaclab/setup.py (without version pinning)
  • Removed custom installation script tools/installation/install_omni_client_packman.py that downloaded and manually installed from CloudFront
  • Removed manual pip install from artifactory in installation flow

Observations:

  • The previous packman script installed version 2.68.1, but the new dependency lacks version pinning which could lead to automatic upgrades
  • The change simplifies the installation process and aligns with standard Python packaging practices
  • omni.client is imported and used in source/isaaclab/isaaclab/utils/assets.py for Nucleus Server file operations

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - simplifies installation by using standard PyPI distribution
  • The changes correctly migrate from a custom installation script to standard pip-based installation. The implementation is straightforward and aligns with best practices. Score is 4 instead of 5 due to lack of version pinning on omniverseclient, which could introduce compatibility issues if breaking changes occur in newer versions. The previous installation used version 2.68.1 explicitly.
  • Pay attention to source/isaaclab/setup.py - consider pinning omniverseclient version to maintain consistency with previous installation (2.68.1)

Important Files Changed

Filename Overview
isaaclab.sh Adds PIP_EXTRA_INDEX_URL for pypi.nvidia.com and removes manual omniverseclient installation, simplifying dependency management
source/isaaclab/setup.py Adds omniverseclient to dependencies without version pinning; may cause compatibility issues if breaking changes occur
tools/installation/install_omni_client_packman.py Deleted custom installation script as omniverseclient is now available from pypi.nvidia.com

Sequence Diagram

sequenceDiagram
    participant User
    participant isaaclab.sh
    participant pip
    participant PyPI_NVIDIA as pypi.nvidia.com
    participant setup.py

    User->>isaaclab.sh: Run installation script
    isaaclab.sh->>isaaclab.sh: Export PIP_EXTRA_INDEX_URL=https://pypi.nvidia.com
    isaaclab.sh->>isaaclab.sh: Export PIP_FIND_LINKS=https://py.mujoco.org/
    isaaclab.sh->>isaaclab.sh: ensure_cuda_torch()
    Note over isaaclab.sh: Removed: install_omni_client_packman.py call
    Note over isaaclab.sh: Removed: pip install from artifactory
    isaaclab.sh->>pip: Install isaaclab packages
    pip->>setup.py: Read dependencies
    setup.py-->>pip: Return INSTALL_REQUIRES (includes omniverseclient)
    pip->>PyPI_NVIDIA: Request omniverseclient package
    PyPI_NVIDIA-->>pip: Return omniverseclient (latest version)
    pip->>pip: Install omniverseclient and dependencies
    pip-->>isaaclab.sh: Installation complete
    isaaclab.sh-->>User: Setup complete
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Comments (1)

  1. source/isaaclab/setup.py, line 45 (link)

    style: Consider pinning to version 2.68.1 (the version used in the previous packman installation) to ensure consistent behavior and prevent unexpected breaking changes

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@kellyguo11 kellyguo11 merged commit d941d57 into isaac-sim:dev/newton Dec 17, 2025
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant