-
Notifications
You must be signed in to change notification settings - Fork 119
SG-39245 Refactor git descriptors #913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #913 +/- ##
==========================================
- Coverage 79.78% 73.76% -6.02%
==========================================
Files 198 198
Lines 20773 20818 +45
==========================================
- Hits 16574 15357 -1217
- Misses 4199 5461 +1262 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the git descriptors to reduce network load and improve performance by replacing full repository clones with remote queries and caching. Key changes include:
- Switching from “git clone” to “git ls-remote” when fetching remote information.
- Adding version pattern matching, get_latest support for git branch/tag descriptors.
- Implementing a caching mechanism to prevent redundant processing.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/descriptor_tests/test_io_descriptors.py | Updated the version attribute type for consistent string handling. |
| tests/descriptor_tests/test_git.py | Updated expected commit/tag values to align with the refactored descriptor logic. |
| tests/descriptor_tests/test_downloadables.py | Replaced shutil.move with a copytree/rmtree approach with onerror callback. |
| tests/descriptor_tests/test_descriptors.py | Updated command validation to use the new _get_git_clone_commands method. |
| python/tank/util/filesystem.py | Added a writable check and chmod call when the destination file is read-only. |
| python/tank/descriptor/io_descriptor/git_tag.py | Improved tag fetching logic with refined exception handling and clearer logging. |
| python/tank/descriptor/io_descriptor/git_branch.py | Made the “version” field optional and streamlined branch-related git commands. |
| python/tank/descriptor/io_descriptor/git.py | Reworked internal git command executions, introduced _normalize_path, and implemented a caching metaclass for git descriptors. |
Comments suppressed due to low confidence (1)
python/tank/descriptor/io_descriptor/git.py:70
- The cache timeout calculation uses time()/100 and a modulus of 2, resulting in a cache window of approximately 200 seconds, which does not match the documented 2 minute validity. Consider reviewing and adjusting the computation to accurately reflect the intended duration.
now = int(time() / 100)
a84ff90 to
608c0c0
Compare
608c0c0 to
756823f
Compare
756823f to
8ddde7e
Compare
Hi there,
I was looking at my temp folder while doing some tests with the tk-core, and I saw that each time I reload an engine, a massive amount of temporary clone folders were created (associated with a big network load). By digging into the source code of git descriptors, I found that they clone repos each and every time they needs to execute a command in it.
So here is a PR with a big refactoring of the git, git_tag and git_branch descriptors:
git clonewithgit ls-remotewhen fetching tags/commit values, checking remote accessget_latestsupport for git branch, and improve the logic for git tag