SG-43167 Add flow_am_project_id to Toolkit Context#1104
Open
stevelittlefish wants to merge 47 commits into
Open
SG-43167 Add flow_am_project_id to Toolkit Context#1104stevelittlefish wants to merge 47 commits into
stevelittlefish wants to merge 47 commits into
Conversation
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>
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>
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>
- 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>
Adds a Flow/MEDM authentication path that triggers proactively during
ToolkitManager bootstrap when the resolved project is "AM-ready" (project
field sg_flow_am_id is set). On the silent path (keyring or refresh
token), no UI surfaces; otherwise a browser PKCE flow opens before the
engine starts.
Sourced from the tk-framework-flowam PoC, which will be deleted in time.
* python/tank_vendor/adsk_auth/ vendored PKCE + keyring helper
(PyJWT + keyring as new third-party
deps, added to per-Python pkgs.zip
via requirements/<py>/requirements.txt)
* python/tank/authentication/flow_auth/
init_authentication(), get_access_token(), check_token_expiry()
FlowAuthSettings + resolve_flow_auth_settings() — defaults + env-var
overrides (TK_FLOW_AUTH_APPLICATION_ID/BASE_URL/CALLBACK_URL)
FlowAuthError / FlowAuthConfigurationError
AM_READY_PROJECT_FIELD (single point of truth — currently
sg_flow_am_id pending confirmation from Julien)
* python/tank/bootstrap/manager.py
_resolve_project_id() extracted from _get_configuration
_check_and_trigger_am_auth() new hook called right before
_get_updated_configuration returns. Configuration errors raise
TankBootstrapError; runtime auth failures are logged and swallowed
unless TK_FLOW_AUTH_REQUIRED=1.
Tests: 29 new (16 flow_auth unit + 13 bootstrap hook), all existing
bootstrap (103) and authentication (101) tests still pass.
FlowAuthenticationHandler from flowam is intentionally dropped — out
of scope for this ticket; tk-core only triggers auth here, it does not
own a GQL handler.
TODOs flagged in code for follow-up: confirm AM-ready field name and
real production APS values (application_id, base_url, callback_url) with
Julien Langlois before release. pkgs.zip regeneration is a release-time
step and not included in this commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- 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>
…nto SG-43166-integrate-medm-auth
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add Sphinx-style type info to _resolve_project_id and _check_and_trigger_am_auth - Change _check_and_trigger_am_auth to accept entity instead of project_id, merging the ShotGrid AM-ready field fetch with project resolution to save one API round-trip (deep-field notation for the general entity case) - Drop the redundant _resolve_project_id call in _get_updated_configuration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sertion Regenerated pkgs.zip, frozen_requirements.txt, and certs for Python 3.7, 3.9, 3.10, 3.11, and 3.13. Also fixed update_python_packages.py to count .dist-info directories instead of package directories when asserting install completeness, since namespace packages (ruamel.yaml, jaraco.*) share parent directories and caused a false assertion failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Bind the PKCE callback HTTP server to 127.0.0.1 / ::1 instead of 0.0.0.0 / :: to prevent exposing the OAuth redirect endpoint on the local network. Port-in-use probes updated to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Call server.shutdown() + server.server_close() in a finally block in web_authenticate() so the bound port and server thread are released once the auth code is obtained, on timeout, or on error — rather than persisting for the lifetime of the host process. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Julien Langlois <16244608+julien-lang@users.noreply.github.com>
This reverts commit 091db05.
Expose the FlowAM project ID (sg_flow_am_id ShotGrid field) as a first-class property on the Context class. The value is fetched lazily from ShotGrid on first access and then cached, so non-AM projects pay no extra cost. It is included in to_dict()/from_dict() serialization so it survives cross-process context handoffs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
stevelittlefish
changed the base branch from
master
to
SG-43166-integrate-medm-auth
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
+ Coverage 79.56% 79.61% +0.05%
==========================================
Files 204 204
Lines 20919 20927 +8
==========================================
+ Hits 16644 16661 +17
+ Misses 4275 4266 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Guard the new context-based AM check against site-level bootstraps where entity is None, which would otherwise crash before reaching _trigger_am_auth's own None guard. Update tests to match the renamed _trigger_am_auth method and the restructured AM-readiness check (now at the call site via ctx.flow_am_project_id). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
config.get_tk_instance() requires a fully serializable user object which is not available mid-bootstrap. Query sg_flow_am_id via self._sg_connection instead, using the existing _resolve_project_id helper to keep it clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yungsiow
reviewed
Jun 4, 2026
carlos-villavicencio-adsk
approved these changes
Jun 4, 2026
julien-lang
reviewed
Jun 4, 2026
- The original solution (caching in private __flow_am_project_id var in context object) didn't seem to work as intended. The project query ended up running multiple times rather than the value being cached and persisting. - Setting the value on the context.project dictionary works and makes the value persist - Changed the flow_am_project_id property to read from context.project, so we still get the benefit of the nicer interface - Removed unnecessary context.__flow_am_project_id which is now unused
- Fix KeyError in flow_am_project_id property: use .get() so non-AM projects return None instead of raising on a missing dict key - Fix NameError in Context._from_dict: remove stale _FLOW_AM_ID_NOT_FETCHED sentinel; restore sg_flow_am_id onto ctx.project from serialized data so serialize/deserialize round-trips are preserved - Fix _trigger_am_auth: add entity is None guard matching the documented no-op behaviour - Update TestContextFlowAmProjectId tests to match the new design where sg_flow_am_id is read directly from the project dict rather than lazy-fetched from ShotGrid Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yungsiow
reviewed
Jun 5, 2026
yungsiow
reviewed
Jun 5, 2026
yungsiow
reviewed
Jun 5, 2026
The merge of master into this branch swapped the bodies of _get_configuration and _check_and_trigger_am_auth, and left the superseded _trigger_am_auth method in place. - Remove _trigger_am_auth (superseded by master's combined _check_and_trigger_am_auth) - Restore _check_and_trigger_am_auth with master's correct body (AM-ready check + auth trigger in one method) - Restore _get_configuration with the correct config resolution body - Simplify _get_updated_configuration tail to call _check_and_trigger_am_auth directly - Update test_manager_flow_auth.py to test _check_and_trigger_am_auth - Remove debug print statement from tank/__init__.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yungsiow
reviewed
Jun 5, 2026
| source_entity=data.get("source_entity"), | ||
| ) | ||
| if ctx.project is not None and "flow_am_project_id" in data: | ||
| ctx.project[flow_auth.AM_READY_PROJECT_FIELD] = data["flow_am_project_id"] |
Contributor
There was a problem hiding this comment.
I feel like we should check if this key already exists on the project object before overwriting it with flow_am_project_id. Is it possible that during serialization the project object has the value stored, but the property was set to None? Since the property is read from the project object, we should always consider that the first source of truth.
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
4 participants
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.
Summary
flow_am_project_idas a first-class property on theContextclasssg_flow_am_idfield on the ShotGrid project entityto_dict()/from_dict()serialization so it survives cross-process context handoffs (e.g. DCC launch)Test plan
tests/core_tests/test_context.py::TestContextFlowAmProjectId- 9 new tests covering: non-AM project, empty context, AM-enabled project, caching, serialization roundtrip, constructor,to_dict, deepcopytests/core_tests/test_context.py::TestSerialize::test_dict_cleanup- updated to includeflow_am_project_idin expected output🤖 Generated with Claude Code