Skip to content

Conversation

@eduardoChaucaGallegos
Copy link
Contributor

@eduardoChaucaGallegos eduardoChaucaGallegos commented Dec 31, 2024

This pull request introduces a major overhaul to the way third-party Python dependencies are managed and loaded in the project, focusing on the tank_vendor mechanism. The changes centralize dependency management using versioned pkgs.zip files, implement a robust dynamic import hook for the tank_vendor namespace, and improve documentation and maintainability. Legacy or redundant scripts and files are removed, and package-specific patches are handled more cleanly.

Dependency Management and Loading Redesign:

  • Introduced a new dynamic loading mechanism in tank_vendor/__init__.py that auto-discovers and loads third-party packages from version-specific pkgs.zip files, supports lazy aliasing for the tank_vendor.* namespace, and applies package-specific patches (e.g., for shotgun_api3 SSL certificates). This includes a custom meta path finder and robust error handling for missing or invalid ZIP files.
  • Updated the documentation in developer/README.md to describe the new requirements folder structure, the workflow for updating bundled dependencies, and the rationale behind using pkgs.zip files for consistent and reproducible environments. [1] [2]

Import and Compatibility Improvements:

  • Enhanced the import handler in python/tank/bootstrap/import_handler.py to correctly skip SourceFileLoader for modules inside ZIP files, ensuring compatibility with the new packaging approach.
  • Removed the legacy upgrade_pyyaml.py script, as dependency upgrades are now handled via the new requirements workflow and automation.

Cleanup and Maintenance:

  • Removed redundant or now-unmanaged files from vendored packages, such as packaging/__init__.py and packaging/_structures.py, as these are now managed within the ZIP-based dependency system. [1] [2]
  • Updated copyright years and added detailed module-level docstrings.

These changes modernize and streamline dependency management for the project, making it more maintainable and robust across Python versions and environments.

Resolves #914

@julien-lang julien-lang requested a review from a team January 6, 2025 16:29
@eduardoChaucaGallegos eduardoChaucaGallegos marked this pull request as draft January 28, 2025 17:30
@codecov
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.79%. Comparing base (67de82a) to head (c92a72b).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #996   +/-   ##
=======================================
  Coverage   79.79%   79.79%           
=======================================
  Files         198      198           
  Lines       20770    20760   -10     
=======================================
- Hits        16573    16566    -7     
+ Misses       4197     4194    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@triley13
Copy link

Hi @triley13, sorry, no, we don't have an ETA. We had to postpone this PR because of new priorities. We still plan to merge it eventually but not in the next few weeks.

Is this something you need? Feel free to share how this change could help you.

Right now we use Rez to manage our internal libraries. This helps to keep our repo clean of duplicating libraries and implementing CICD to package the site-packages with our internal libs. With this, we can pull from our py registry (github), to manage our library injections.

Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

LGTM. The init.py file it's too big in my opinion. It would be great to have a better organization/modularization there. Maybe we can consider using a main.py file? These are just thoughts.

Besides this, I think we should start testing this throughoutly.

Copy link
Member

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

A few changes please. Also, I agree with all comments from Charlie so could you address them please?

And could you add some unit tests validating the import and handle of third partiy libs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants