SG-42277 Fix get_currently_running_api_version() under pip install#1096
Open
stevelittlefish wants to merge 5 commits intomasterfrom
Open
SG-42277 Fix get_currently_running_api_version() under pip install#1096stevelittlefish wants to merge 5 commits intomasterfrom
stevelittlefish wants to merge 5 commits intomasterfrom
Conversation
…gration tests The function is defined in tank.pipelineconfig_utils and is not re-exported at the top of the tank/sgtk module, so sgtk.get_currently_running_api_version raises AttributeError. Call it via sgtk.pipelineconfig_utils instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion test The integration test runner prepends the source tree's python/ directory to PYTHONPATH, which the test's subprocess inherits. This shadows the venv's pip-installed sgtk with the source tree, causing _get_version_from_manifest to pick up the source repo's info.yml (version: "HEAD") instead of exercising the importlib.metadata fallback. Drop PYTHONPATH from the subprocess env so the venv's site-packages takes precedence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1096 +/- ##
==========================================
- Coverage 79.43% 79.43% -0.01%
==========================================
Files 198 198
Lines 20749 20756 +7
==========================================
+ Hits 16482 16487 +5
- Misses 4267 4269 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
julien-lang
requested changes
May 8, 2026
| ) | ||
| return _get_version_from_manifest(info_yml_path) | ||
| version = _get_version_from_manifest(info_yml_path) | ||
| if version == "unknown": |
Member
There was a problem hiding this comment.
Not great that we compare to string unknown. It would be better to use Python None.
Also, can we return success sooner. (reverse the condition).
Comment on lines
+451
to
+452
| except Exception: | ||
| pass |
Member
There was a problem hiding this comment.
Please bring specific expcetion if possible
| # Fall back to the installed distribution metadata; PEP 440 strips the | ||
| # leading 'v', so re-add it to match the info.yml convention. | ||
| try: | ||
| from importlib.metadata import version as _dist_version |
Member
There was a problem hiding this comment.
can we import at the beginning of the file?
import importlib.metadataThen
return importlib.metadata.version("sgtk")
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
When tk-core is pip-installed into a venv, the package lands in a flat
site-packages/tank/layout. The native install layout assumed by the code(
install/core/python/tank/withinfo.ymltwo levels up fromtank/) isabsent, so
pipelineconfig_utils.get_currently_running_api_version()resolves
info.ymlto a non-existent<venv>/Lib/info.yml,_get_version_from_manifestswallows the error, and"unknown"is returned.This silently breaks
Sgtk.versionand any code path that compares thecurrently-running core version against a config's required core version
(e.g.
PipelineConfiguration.__init__atpipelineconfig.py:73).Related to the broader pip-install layout cleanup in #1091.
Fix
Add an
importlib.metadata.version("sgtk")fallback inget_currently_running_api_version(). Theinfo.ymlread remains theprimary source — only when it returns
"unknown"do we consult theinstalled sgtk distribution metadata. PEP 440 strips the leading
v, sothe fallback re-adds it to match the
info.ymlformat and the docstring'sdocumented return shape (
'v1.2.3').Behavior:
info.ymlpresent?sgtkdist-info present?"HEAD","v0.23.8")"v" + dist_version(e.g."v0.23.8")"unknown"(preserves original contract)_get_version_from_manifestandget_core_api_version(core_install_root)are unchanged. The latter has different semantics — "what version is at
this core install root?" — where
"unknown"is the correct answer whenthe manifest at that root is missing.
Out of scope (still broken under pip; can be addressed separately):
Sgtk.documentation_url, core hook resolution, theTANK_CURRENT_PCskipin
tank/__init__.py, andsetup_project_core's binary copying. Each ofthose reads files (
hooks/,schema/,setup/) that aren't shipped tosite-packages/.Testing
Three layers:
tests/core_tests/test_pipelineconfig_utils.py—new
TestGetCurrentlyRunningApiVersionclass with three mocked tests:manifest present (fallback not consulted), manifest missing → fallback
succeeds, both missing →
"unknown". Hermetic, fast.tests/integration_tests/pip_install.py— extendedtest_pip_install_and_importto assertget_currently_running_api_version()returns a non-"unknown"vX.Y.Zstring after a real
pip installinto a fresh venv.tests/integration_tests/pip_install_bootstrap.py— addedtest_get_currently_running_api_version_in_simulated_pip_layoutasserting the function returns a string and does not raise in the
simulated flat layout. Assertion is intentionally permissive:
importlib.metadatareads from the runtime Python'ssite-packages,not the
sys.path-prepended simulated copy, so a developer who alsohas sgtk pip-installed in their dev Python will get the dist version
rather than
"unknown".