Skip to content

SG-42277 Fix get_currently_running_api_version() under pip install#1096

Open
stevelittlefish wants to merge 5 commits intomasterfrom
ticket/SG-42277_unresolved_version_when_pip_installed
Open

SG-42277 Fix get_currently_running_api_version() under pip install#1096
stevelittlefish wants to merge 5 commits intomasterfrom
ticket/SG-42277_unresolved_version_when_pip_installed

Conversation

@stevelittlefish
Copy link
Copy Markdown

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/ with info.yml two levels up from tank/) is
absent, so pipelineconfig_utils.get_currently_running_api_version()
resolves info.yml to a non-existent <venv>/Lib/info.yml,
_get_version_from_manifest swallows the error, and "unknown" is returned.

This silently breaks Sgtk.version and any code path that compares the
currently-running core version against a config's required core version
(e.g. PipelineConfiguration.__init__ at
pipelineconfig.py:73).

Related to the broader pip-install layout cleanup in #1091.

Fix

Add an importlib.metadata.version("sgtk") fallback in
get_currently_running_api_version(). The info.yml read remains the
primary source — only when it returns "unknown" do we consult the
installed sgtk distribution metadata. PEP 440 strips the leading v, so
the fallback re-adds it to match the info.yml format and the docstring's
documented return shape ('v1.2.3').

Behavior:

Install layout info.yml present? sgtk dist-info present? Returned value
Source / native yes n/a manifest value (e.g. "HEAD", "v0.23.8")
Pip install no yes "v" + dist_version (e.g. "v0.23.8")
Bare copy / no metadata no no "unknown" (preserves original contract)

_get_version_from_manifest and get_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 when
the manifest at that root is missing.

Out of scope (still broken under pip; can be addressed separately):
Sgtk.documentation_url, core hook resolution, the TANK_CURRENT_PC skip
in tank/__init__.py, and setup_project_core's binary copying. Each of
those reads files (hooks/, schema/, setup/) that aren't shipped to
site-packages/.

Testing

Three layers:

  • Unit tests in tests/core_tests/test_pipelineconfig_utils.py
    new TestGetCurrentlyRunningApiVersion class with three mocked tests:
    manifest present (fallback not consulted), manifest missing → fallback
    succeeds, both missing → "unknown". Hermetic, fast.
  • tests/integration_tests/pip_install.py — extended
    test_pip_install_and_import to assert
    get_currently_running_api_version() returns a non-"unknown" vX.Y.Z
    string after a real pip install into a fresh venv.
  • tests/integration_tests/pip_install_bootstrap.py — added
    test_get_currently_running_api_version_in_simulated_pip_layout
    asserting the function returns a string and does not raise in the
    simulated flat layout. Assertion is intentionally permissive:
    importlib.metadata reads from the runtime Python's site-packages,
    not the sys.path-prepended simulated copy, so a developer who also
    has sgtk pip-installed in their dev Python will get the dist version
    rather than "unknown".

Note: Draft. The first commit on this branch contains the new tests
only — the fix in pipelineconfig_utils.py will land in a follow-up
commit. CI on this commit is expected to fail
test_falls_back_to_dist_metadata_when_manifest_missing and the
extended test_pip_install_and_import, demonstrating that the new
tests catch the bug. CI should go green once the fix is added.

stevelittlefish and others added 4 commits May 8, 2026 12:22
…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
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.43%. Comparing base (5d4ea39) to head (e0db8bc).

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     
Flag Coverage Δ
Linux 78.90% <100.00%> (+<0.01%) ⬆️
Python-3.10 79.25% <100.00%> (+<0.01%) ⬆️
Python-3.11 79.15% <100.00%> (+<0.01%) ⬆️
Python-3.13 79.15% <100.00%> (+<0.01%) ⬆️
Python-3.9 79.21% <100.00%> (-0.01%) ⬇️
Windows 78.93% <100.00%> (-0.01%) ⬇️
macOS 78.91% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stevelittlefish stevelittlefish marked this pull request as ready for review May 8, 2026 13:56
@stevelittlefish stevelittlefish requested a review from a team May 8, 2026 15:12
Copy link
Copy Markdown
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

)
return _get_version_from_manifest(info_yml_path)
version = _get_version_from_manifest(info_yml_path)
if version == "unknown":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we import at the beginning of the file?

            import importlib.metadata

Then

           return importlib.metadata.version("sgtk")

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.

3 participants