From 89cb551dbb59c768c034986ac4039cddc4ae13af Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Thu, 21 May 2026 13:12:05 +1000 Subject: [PATCH] Run Autodesk upload/download on a worker thread The progress dialog was the only connector window not driven by a Tk event loop: the handler created it, then blocked inline in httpx I/O. On Windows CTkToplevel withdraws itself at construction and re-shows via a delayed after() callback, which never fires without a running loop, so the progress window stayed invisible for the whole transfer. Add run_with_progress(): the blocking work runs on a daemon thread while the main thread pumps the Tk loop and shows the dialog. Progress reports are coalesced and marshalled back to the UI thread via _ProgressBridge, and worker exceptions are re-raised on the main thread, preserving the JSON-RPC error path. All eight upload/download handlers converted. Co-Authored-By: Claude Opus 4.7 --- .../bonsaiviewer_autodesk/connector.py | 69 ++++++----- .../bonsaiviewer_autodesk/ui.py | 108 +++++++++++++++++- .../tests/test_connector.py | 11 +- 3 files changed, 152 insertions(+), 36 deletions(-) diff --git a/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/connector.py b/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/connector.py index 8fd5860cef..2e785df620 100644 --- a/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/connector.py +++ b/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/connector.py @@ -8,7 +8,7 @@ from typing import Any, Callable from bonsaiviewer_autodesk import cache, settings from bonsaiviewer_autodesk.autodesk import ApsClient, AuthSessionService, KeyringTokenStore from bonsaiviewer_autodesk.rpc import JSONRPC_INTERNAL_ERROR, JSONRPC_INVALID_PARAMS, RpcError -from bonsaiviewer_autodesk.ui import BrowseDialog, SettingsDialog, progress_dialog, prompt_for_filename +from bonsaiviewer_autodesk.ui import BrowseDialog, SettingsDialog, prompt_for_filename, run_with_progress ApsProgress = Callable[[str, "int | None", "int | None", "int | None"], None] @@ -122,15 +122,17 @@ class AutodeskConnector: hub = chosen["hub"] project = chosen["project"] entry = chosen["entries"][0] - with progress_dialog("Downloading project") as report: - path = self._download_ifcfed( + path = run_with_progress( + "Downloading project", + lambda report: self._download_ifcfed( aps=aps, hub_id=hub["id"], project_id=project["id"], item_id=entry["id"], display_name=entry["display_name"], progress=_download_callback(report), - ) + ), + ) return {"path": str(path)} # ---- pull_ifcfed -------------------------------------------------------- @@ -142,15 +144,17 @@ class AutodeskConnector: project_id = _require_string(manifest, "project_id") item_id = _require_string(manifest, "item_id") display_name = manifest.get("display_name") or item_id - with progress_dialog("Downloading project") as report: - path = self._download_ifcfed( + path = run_with_progress( + "Downloading project", + lambda report: self._download_ifcfed( aps=aps, hub_id=hub_id, project_id=project_id, item_id=item_id, display_name=display_name, progress=_download_callback(report), - ) + ), + ) return {"path": str(path)} def _download_ifcfed( @@ -193,9 +197,10 @@ class AutodeskConnector: def pull_models(self, params: Any) -> list[dict[str, Any] | None]: _, aps = self._require_aps() models = _require_array(params, "params") - results: list[dict[str, Any] | None] = [] total = len(models) - with progress_dialog("Downloading models") as report: + + def work(report: Report) -> list[dict[str, Any] | None]: + results: list[dict[str, Any] | None] = [] for index, model in enumerate(models): callback = _download_callback(report, index=index + 1, total=total) try: @@ -207,7 +212,9 @@ class AutodeskConnector: print(f"pull_models[{index}] skipped: {exc}", file=sys.stderr) traceback.print_exc(file=sys.stderr) results.append(None) - return results + return results + + return run_with_progress("Downloading models", work) def _resolve_model( self, @@ -257,10 +264,10 @@ class AutodeskConnector: hub = chosen["hub"] project = chosen["project"] entries = chosen["entries"] - - results: list[dict[str, Any]] = [] total = len(entries) - with progress_dialog("Downloading models") as report: + + def work(report: Report) -> list[dict[str, Any]]: + results: list[dict[str, Any]] = [] for index, entry in enumerate(entries): callback = _download_callback(report, index=index + 1, total=total) try: @@ -274,7 +281,9 @@ class AutodeskConnector: continue if result is not None: results.append(result) - return results + return results + + return run_with_progress("Downloading models", work) def _download_picked_model( self, @@ -338,14 +347,16 @@ class AutodeskConnector: if not file_name.lower().endswith(".ifcfed"): file_name = file_name + ".ifcfed" - with progress_dialog("Uploading project") as report: - uploaded = aps.upload_file_to_folder( + uploaded = run_with_progress( + "Uploading project", + lambda report: aps.upload_file_to_folder( project["id"], folder["id"], local_path, display_name=file_name, progress=_upload_callback(report), - ) + ), + ) directory = cache.prepare_sole_child_dir(cache.ifcfed_dir(project["id"], uploaded["item_id"])) cached_path = directory / file_name @@ -390,14 +401,16 @@ class AutodeskConnector: raise RpcError(JSONRPC_INTERNAL_ERROR, f"Cannot resolve parent folder for item '{item_id}'.") file_name = manifest.get("display_name") or item.get("display_name") or local_path.name - with progress_dialog("Uploading project") as report: - uploaded = aps.upload_file_to_folder( + uploaded = run_with_progress( + "Uploading project", + lambda report: aps.upload_file_to_folder( project_id, folder_id, local_path, display_name=file_name, progress=_upload_callback(report), - ) + ), + ) directory = cache.prepare_sole_child_dir(cache.ifcfed_dir(project_id, uploaded["item_id"])) cached_path = directory / file_name @@ -436,14 +449,16 @@ class AutodeskConnector: if not file_name: raise RpcError(JSONRPC_INTERNAL_ERROR, "User cancelled save to cloud.") - with progress_dialog("Uploading model") as report: - uploaded = aps.upload_file_to_folder( + uploaded = run_with_progress( + "Uploading model", + lambda report: aps.upload_file_to_folder( project["id"], folder["id"], local_path, display_name=file_name, progress=_upload_callback(report), - ) + ), + ) directory = cache.model_dir(project["id"], uploaded["item_id"], uploaded["version_id"]) cache.prepare_sole_child_dir(directory) @@ -488,14 +503,16 @@ class AutodeskConnector: raise RpcError(JSONRPC_INTERNAL_ERROR, f"Cannot resolve parent folder for item '{item_id}'.") file_name = item.get("display_name") or local_path.name - with progress_dialog("Uploading model") as report: - uploaded = aps.upload_file_to_folder( + uploaded = run_with_progress( + "Uploading model", + lambda report: aps.upload_file_to_folder( project_id, folder_id, local_path, display_name=file_name, progress=_upload_callback(report), - ) + ), + ) directory = cache.model_dir(project_id, uploaded["item_id"], uploaded["version_id"]) cache.prepare_sole_child_dir(directory) diff --git a/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/ui.py b/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/ui.py index 7a58e616c8..3981c2e514 100644 --- a/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/ui.py +++ b/src/bonsaiviewer-autodesk/bonsaiviewer_autodesk/ui.py @@ -1,8 +1,10 @@ from __future__ import annotations +import threading +import time import tkinter as tk from tkinter import ttk -from typing import TYPE_CHECKING, Any, Callable, Literal +from typing import TYPE_CHECKING, Any, Callable, Literal, TypeVar import customtkinter as ctk @@ -226,9 +228,107 @@ class _ProgressContext: pass -def progress_dialog(message: str) -> _ProgressContext: - """Standalone progress dialog usable outside the browse picker.""" - return _ProgressContext(None, message) +T = TypeVar("T") + +# A progress sink: (phase, message, percent, detail) -> None. +Report = Callable[[str, str, "int | None", "str | None"], None] + + +class _ProgressBridge: + """Thread-safe hand-off of the latest progress report to the UI thread. + + ``work`` runs on a worker thread and must never touch Tk; it calls + :meth:`report`, which only stashes the most recent update. The main thread + drains it via :meth:`take` and applies it to the dialog. Intermediate + updates are coalesced — only the latest matters for a progress bar. + """ + + def __init__(self) -> None: + self._lock = threading.Lock() + self._pending: tuple[str, str, int | None, str | None] | None = None + + def report( + self, + phase: str, + message: str, + percent: int | None = None, + detail: str | None = None, + ) -> None: + with self._lock: + self._pending = (phase, message, percent, detail) + + def take(self) -> tuple[str, str, int | None, str | None] | None: + with self._lock: + pending, self._pending = self._pending, None + return pending + + +def run_with_progress( + message: str, + work: Callable[[Report], T], + *, + parent: tk.Misc | None = None, +) -> T: + """Show a ProgressDialog and run ``work`` on a worker thread. + + Tkinter is single-threaded and only repaints while its event loop runs, so + a long upload/download executed inline would freeze the dialog. Worse, on + Windows ``CTkToplevel`` withdraws itself at construction and re-shows via a + delayed ``after()`` callback — without a running loop that callback never + fires and the window stays invisible for the whole transfer. + + So the blocking ``work`` runs on a background thread while the main thread + pumps the Tk event loop here. ``work`` receives a thread-safe ``report`` + callback; its updates are marshalled back to the UI thread. Returns work's + result, or re-raises (on the main thread) whatever exception it raised. + """ + root = ensure_tk_app() + dialog = ProgressDialog(message, parent) + bridge = _ProgressBridge() + outcome: dict[str, Any] = {} + + def runner() -> None: + try: + outcome["value"] = work(bridge.report) + except BaseException as exc: # noqa: BLE001 - re-raised on the main thread + outcome["error"] = exc + + thread = threading.Thread(target=runner, name="autodesk-progress", daemon=True) + thread.start() + + try: + while thread.is_alive(): + pending = bridge.take() + try: + if pending is not None: + dialog.report(*pending) + else: + root.update() + except tk.TclError: + break + time.sleep(0.03) + thread.join() + final = bridge.take() + if final is not None: + try: + dialog.report(*final) + except tk.TclError: + pass + finally: + try: + dialog.withdraw() + dialog.destroy() + except tk.TclError: + pass + try: + root.update_idletasks() + root.update() + except tk.TclError: + pass + + if "error" in outcome: + raise outcome["error"] + return outcome["value"] # --- browse ------------------------------------------------------------------ diff --git a/src/bonsaiviewer-autodesk/tests/test_connector.py b/src/bonsaiviewer-autodesk/tests/test_connector.py index c6f1394453..6905108c63 100644 --- a/src/bonsaiviewer-autodesk/tests/test_connector.py +++ b/src/bonsaiviewer-autodesk/tests/test_connector.py @@ -1,12 +1,11 @@ """Tests for the RPC handlers (``bonsaiviewer_autodesk.connector``). The non-interactive handlers are exercised against a fake ``ApsClient`` so no -network or GUI is touched; ``progress_dialog`` is stubbed out. +network or GUI is touched; ``run_with_progress`` is stubbed out. """ from __future__ import annotations -from contextlib import contextmanager from pathlib import Path import pytest @@ -18,9 +17,9 @@ from bonsaiviewer_autodesk.rpc import RpcError # --- fakes / fixtures -------------------------------------------------------- -@contextmanager -def _fake_progress(_message): - yield lambda *_args, **_kwargs: None +def _fake_run_with_progress(_message, work, *, parent=None): + """Run ``work`` inline with a no-op report — no worker thread, no GUI.""" + return work(lambda *_args, **_kwargs: None) class FakeAps: @@ -58,7 +57,7 @@ class FakeAps: @pytest.fixture def make_connector(config_dir, cache_dir, monkeypatch): """Build an AutodeskConnector wired to a fake ApsClient.""" - monkeypatch.setattr(connector, "progress_dialog", _fake_progress) + monkeypatch.setattr(connector, "run_with_progress", _fake_run_with_progress) def _make(aps: FakeAps) -> connector.AutodeskConnector: conn = connector.AutodeskConnector()