Skip to content

SG-43217 Integrate Flow Data SDK as a vendored beta library#1098

Open
stevelittlefish wants to merge 11 commits into
masterfrom
ticket/SG-43217/flow-data-sdk
Open

SG-43217 Integrate Flow Data SDK as a vendored beta library#1098
stevelittlefish wants to merge 11 commits into
masterfrom
ticket/SG-43217/flow-data-sdk

Conversation

@stevelittlefish
Copy link
Copy Markdown
Contributor

@stevelittlefish stevelittlefish commented May 14, 2026

Summary

  • Adds requirements/any/ for Python-version-independent vendor zips, sibling to the existing per-Python-version pkgs.zip directories.
  • Teaches python/tank_vendor/__init__.py to auto-discover and load every *.zip in requirements/any/ alongside pkgs.zip, aliasing each top-level package under tank_vendor.* via the existing meta-path finder.
  • Drops in flow_data_sdk-beta.zip as the first such shared vendor — the Autodesk Flow Data SDK at 2.0.37, used by Toolkit code as from tank_vendor import flow_data_sdk.

Implementation notes

  • The existing pkgs.zip init was extracted into a reusable _load_packages_from_zip helper. pkgs.zip raises RuntimeError only on a wholesale import failure inside the zip; missing or unreadable pkgs.zip is tolerated (warns for unreadable; silent for missing, to support pip-installed setups where dependencies come from the Python environment). Shared zips warn-and-continue throughout.
  • pkgs.zip is loaded first, so per-Python-version pins win on name collision. Collisions warn and skip rather than overwrite.
  • Per-package import failures continue to warn-and-continue. The Flow SDK uses Python 3.10+ syntax (types.UnionType, typing.TypeAlias); on 3.7/3.9 it will simply be absent from tank_vendor instead of breaking import tank_vendor.
  • No new patches needed — Flow SDK uses the system trust store and ships no on-disk data files. The existing _patch_shotgun_api3_certs is untouched.

Files

Path Change
python/tank_vendor/__init__.py Refactor into _load_packages_from_zip helper; add requirements/any/ glob loop
requirements/any/flow_data_sdk-beta.zip New vendored SDK zip (68 KB)
tests/core_tests/test_tank_vendor.py Extend PACKAGES_TO_TEST (3.10+ gated) + new TestFlowDataSDK class
developer/README.md Document the new requirements/any/ location

Test plan

  • CI green on all supported Python versions (3.7, 3.9, 3.10, 3.11, 3.13)
  • python tests/run_tests.py core_tests/test_tank_vendor.py passes (15 tests; locally verified on 3.10)
  • python tests/run_tests.py core_tests passes with no regressions (481 tests; locally verified on 3.10)
  • On 3.7 / 3.9, import tank_vendor succeeds and emits a single warning that flow_data_sdk isn't available (expected; SDK requires 3.10+)
  • Smoke: python -c "from tank_vendor import flow_data_sdk; print(flow_data_sdk.SDK_VERSION); from tank_vendor.flow_data_sdk import GQLClient; print(GQLClient)" prints 2.0.37 and the GQLClient class

🤖 Generated with Claude Code

stevelittlefish and others added 2 commits May 13, 2026 15:49
Adds requirements/any/ for Python-version-independent vendor zips and
teaches python/tank_vendor/__init__.py to auto-discover and load them
alongside the existing pkgs.zip. Drops in flow_data_sdk-beta.zip as the
first such vendor.

The loader refactor extracts the existing pkgs.zip init into a reusable
_load_packages_from_zip helper. Shared zips load after pkgs.zip so
per-version pins win on name collision; collisions warn and skip rather
than overwrite. Per-package import failures continue to warn-and-continue
(the SDK uses 3.10+ syntax, so it'll simply be absent on 3.7/3.9 instead
of breaking import tank_vendor).

Includes a small _patch_flow_data_sdk_version workaround for an upstream
bug: the SDK's _version.py queries importlib.metadata.version(
"adsk-flow-data") but the published wheel's distribution name is
"flow-data-sdk", so SDK_VERSION otherwise falls back to "local_dev"
even with .dist-info present. The patch is a self-disabling no-op once
upstream is fixed.

Tests cover the new package via PACKAGES_TO_TEST (3.10+ gated) plus a
TestFlowDataSDK class with a dist-info canary that catches future
regressions in the zip's metadata packaging.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Flow Data SDK previously hardcoded the wrong distribution name in
flow_data_sdk/base/_version.py ("adsk-flow-data" instead of the actual
published name "flow-data-sdk"), so importlib.metadata.version() always
raised PackageNotFoundError and SDK_VERSION fell back to "local_dev"
even with .dist-info present in our shared zip.

The new SDK zip ships the upstream one-line fix — _version.py now
queries the correct name and SDK_VERSION resolves to the real version
on its own. The local workaround patch can go.

Removes:
- _patch_flow_data_sdk_version function and its call site in
  tank_vendor/__init__.py (~37 lines)
- The upstream-bug commentary in the test_dist_info_via_importlib_metadata
  docstring (the assertion itself is unchanged and still passes)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.51%. Comparing base (34cb3fb) to head (8381203).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
+ Coverage   79.42%   79.51%   +0.09%     
==========================================
  Files         198      198              
  Lines       20750    20842      +92     
==========================================
+ Hits        16481    16573      +92     
  Misses       4269     4269              
Flag Coverage Δ
Linux 78.99% <ø> (+0.09%) ⬆️
Python-3.10 79.25% <ø> (+<0.01%) ⬆️
Python-3.11 79.15% <ø> (+<0.01%) ⬆️
Python-3.13 79.15% <ø> (+<0.01%) ⬆️
Python-3.9 79.30% <ø> (+0.09%) ⬆️
Windows 78.92% <ø> (+<0.01%) ⬆️
macOS 78.90% <ø> (+<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 and others added 2 commits May 14, 2026 13:37
The required parameter was only meaningful in the late-stage-exception
branch (raise vs warn). The other two failure paths returned False
identically in both modes, so the parameter was mostly dead weight.

Collapse to always-raise for any zip's wholesale load failure, matching
the original pkgs.zip posture. Shared zips in requirements/any/ are now
held to the same standard: a corrupt or import-broken shared zip will
fail import tank_vendor with a clean RuntimeError instead of silently
degrading. Per-package ImportError inside the loop still warns and
skips, so flow_data_sdk being absent on Python 3.7/3.9 (3.10+ syntax)
remains non-fatal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

This comment was marked as resolved.

stevelittlefish and others added 4 commits May 15, 2026 12:07
Fixes a regression in the share_core integration test on Windows /
Python 3.13.

flow_data_sdk's _version.py calls importlib.metadata.version(
"flow-data-sdk") at import time. importlib.metadata iterates sys.path
in order, and FastPath.zip_children() is @lru_cache'd — the cached
FastPath holds an open zipfile.ZipFile to whichever zip it probed.

Previously pkgs.zip was at sys.path[0] and flow_data_sdk-beta.zip at
sys.path[1], so the scan probed pkgs.zip first (no match → cached open
handle), then matched in flow_data_sdk-beta.zip. The lingering handle
on pkgs.zip caused share_core's shutil.move to fail with WinError 32
when relocating install/core on Windows.

Reorder so shared zips end up at sys.path[0], pkgs.zip at sys.path[1].
importlib.metadata then short-circuits on the first probe and never
opens pkgs.zip. pkgs.zip is still loaded into sys.modules first, so
collision precedence is unchanged.

Drop the path_position parameter from _load_packages_from_zip — every
zip is now always inserted at sys.path[0], and the call order in
tank_vendor/__init__.py determines the final sys.path ordering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… zips

The previous reorder commit (f96c7d6) moved the share_core WinError 32
from pkgs.zip to flow_data_sdk-beta.zip — same root cause, different
file. This is the actual fix.

importlib.metadata.FastPath.__new__ is @lru_cache'd. The FastPath
instance for whichever zip importlib.metadata probes is kept alive
forever, and inside FastPath.zip_children() the line
`self.joinpath = zip_path.joinpath` binds a zipfile.Path (with its
underlying open ZipFile) as an instance attribute. The result: the
cache permanently pins an open file handle on every zip that ever
yielded a metadata match.

flow_data_sdk's _version.py triggers this by calling
importlib.metadata.version("flow-data-sdk") at module import time.
The cached FastPath then keeps flow_data_sdk-beta.zip open, which
on Windows blocks share_core's shutil.move(install/core, ...).

Fix: after all zips are loaded, call MetadataPathFinder().invalidate_caches()
to drop FastPath references, then gc.collect() so the underlying
ZipFile.__del__ fires immediately and releases the OS handle.

invalidate_caches is called on an instance, not the class, because it
isn't decorated as @classmethod in older Python versions but takes
`cls` by convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cleanup added in 8c006ca (clearing FastPath cache + gc.collect to
release zipfile handles) is a workaround for Windows' sharing-violation
file-move semantics. Linux and macOS allow moving files with open
handles, so the cleanup is unnecessary there — and it was observed to
break a Linux / Python 3.13 integration test in CI.

Guard with sys.platform == "win32" so non-Windows platforms get the
previous behaviour unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three changes prompted by PR review on #1098:

1. Broaden per-package import catch from ImportError to Exception. The
   inner try/except is best-effort by design (the outer wholesale-failure
   handler still raises). A future shared vendor using PEP 604 unions or
   other syntax-level newness would currently break import tank_vendor
   on older Pythons with a SyntaxError escaping the inner catch;
   widening the except matches the documented intent.

2. Add TestFlowDataSDKAbsentOnOldPython, gated to Python < 3.10. Pins the
   contract that the loader warns and continues when a shared vendor
   fails to import, so the PR's behavioural claim ("on 3.7/3.9 the SDK
   is simply absent") is actually exercised in CI rather than just
   asserted in the description.

3. Soften the misleading "mandatory" label on pkgs.zip in the module
   docstring. Missing pkgs.zip is tolerated to support pip-installed
   tk-core where dependencies come from the environment; the docstring
   for _load_packages_from_zip already says so, but the top-of-file
   summary still claimed otherwise.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@stevelittlefish stevelittlefish requested a review from Copilot May 15, 2026 17:08
@stevelittlefish stevelittlefish marked this pull request as ready for review May 15, 2026 17:09
@stevelittlefish stevelittlefish requested a review from a team May 15, 2026 17:09

This comment was marked as resolved.

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.

Looks good so far.

I'd love to see python-api (shotgun_api3) also in the requirements/any because it's also a python version independent. I'm not suggesting this for the PR, it can be done later.

Comment thread python/tank_vendor/__init__.py
- Filter top-level .dll files in _discover_top_level_packages so a stray
  Windows DLL at the root of a vendor zip is not treated as an
  importable package (Carlos).
- Reword the unreadable-zip warning to acknowledge that affected
  dependencies may still resolve from the Python environment instead of
  implying a guaranteed failure (Copilot).
- Pass RuntimeWarning + stacklevel=2 on per-package import failures so
  they match the other warnings in this module and point at the caller
  (Copilot).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread python/tank_vendor/__init__.py Outdated
Comment thread developer/README.md Outdated
Comment thread python/tank_vendor/__init__.py
Comment thread python/tank_vendor/__init__.py Outdated
Comment thread python/tank_vendor/__init__.py
- Restore the unicode → arrow in the _install_import_hook docstring.
- Wrap the new requirements/any/ paragraph in developer/README.md.
- Restore the Step 1/2/3/4 navigational comments inside
  _load_packages_from_zip that the refactor had stripped.
- Drop the Python<3.8 fallback in _release_importlib_metadata_handles;
  Python 3.7/3.8 compatibility was discontinued after March 2026.
- Reorganise __init__.py so all helper defs (including
  _release_importlib_metadata_handles) come before the MAIN
  INITIALIZATION block, and call the release helper from the main block.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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

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

LGTM!

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.

4 participants