mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-06 07:51:47 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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 ------------------------------------------------------------------
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user