opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-24 16:30:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

LazyToolBox: drop to_panel_view + _index_entry_to_api_dict; let super render

Browse Source 
With ``_tool_panel`` filled with ``LazyTool`` stubs by the seam-driven
walk, the parent ``AbstractToolBox.to_panel_view`` renders the default
view from ``_tool_panel_view_rendered`` (cached at boot by
``_load_tool_panel_views``). For each tool encountered it calls
``ToolSection.to_dict(only_ids=True)`` → ``tool.to_dict(link_details=False)``,
which lands in ``LazyTool.to_dict``'s entry-only fast path. No
materialisation; the lazy-custom rendering is redundant.

``_index_entry_to_api_dict`` goes away with it — the same shape now
comes from ``LazyTool.to_dict(link_details=False)``.

Also drops the leftover ``_populate_tool_registry_from_index`` block
(missed in commit 4f82c319fcc — it referenced ``_tool_section_map``
which no longer exists, and was orphaned the moment ``__init__``
stopped calling it). With its removal, ``ToolSection`` /
``panel_item_types`` are still referenced by the panel post-walk
stamping helpers, but ``EdamToolPanelView`` is now fully unused —
import dropped.

``test_default_panel_view_section_tools_use_id_list`` was asserting
that section ``tools`` lists held only id strings — that was specific
to the lazy fast path. The eager renderer emits
``ToolSectionLabel`` dicts interleaved with id strings (that's what
``ToolSection.to_dict(only_ids=True)`` produces for a section that
contains labels). The test's true intent is to prevent regression
where full *Tool* dicts appear in ``tools``; loosen the assertion to
accept id strings + ``ToolSectionLabel`` dicts but no other shape.

10/11 integration suite still passing; unit and tox clean.
This commit is contained in:
mvdbeek
2026-07-28 17:27:15 +02:00
parent 51042ff543
commit 0b87f78e5b
2 changed files with 13 additions and 204 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/galaxy/tools/lazy_toolbox.py
-196
View File
@@ -43,7 +43,6 @@ from galaxy.tool_util.toolbox.panel import (
panel_item_types,
ToolSection,
)
from galaxy.tool_util.toolbox.views.edam import EdamToolPanelView
from . import (
create_tool_from_source,
ToolBox,
@@ -632,100 +631,6 @@ class LazyToolBox(ToolBox):
fallback_tool_id=stored.tool_id,
)
def _populate_tool_registry_from_index(self) -> None:
"""
Populate _tools_by_id with None placeholders from index.
This allows has_tool() checks to work without loading Tool objects.
The actual Tool objects are loaded on-demand in get_tool().
"""
if self._tool_index is None:
return
# Update index entries with section info from tool_conf.xml
# Build reverse map: short_id -> section_info for faster lookup
if hasattr(self, "_tool_section_map"):
short_id_to_section: dict[str, tuple] = {}
for map_tool_id, mapped_section in self._tool_section_map.items():
# Store exact ID
short_id_to_section[map_tool_id] = mapped_section
# For guids, also store the short tool ID
short_id = extract_short_id_from_guid(map_tool_id)
if short_id and short_id != map_tool_id and short_id not in short_id_to_section:
short_id_to_section[short_id] = mapped_section
else:
short_id_to_section = {}
section_updates = 0
section_info: Optional[tuple]
for tool_id, entry in self._tool_index.entries.items():
section_info = None
# Try exact match first
if tool_id in short_id_to_section:
section_info = short_id_to_section[tool_id]
if section_info:
section_id, section_name = section_info
if section_id and not entry.panel_section_id:
entry.panel_section_id = section_id
entry.panel_section_name = section_name
section_updates += 1
# Store None as placeholder - actual Tool loaded on demand
self._tools_by_id[tool_id] = None # type: ignore[assignment]
# Initialize version tracking. Walk every version we indexed so
# ``has_tool(tool_id, exact=True)`` and version-aware lookups
# behave the same as the eager toolbox (which registers each
# ``Tool`` instance per version).
self._tool_versions_by_id.setdefault(tool_id, {})
versions = self._tool_index.entries_by_version.get(tool_id, {entry.version or "": entry})
for version_key in versions.keys():
if version_key:
self._tool_versions_by_id[tool_id][version_key] = None # type: ignore[assignment]
# Lineage is built lazily by ``LazyLineageMap`` from
# ``_tool_index.entries_by_version`` on first ``get()``; no
# boot-time seeding pass is needed.
# Add to panel if section info available
if entry.panel_section_id and entry.panel_section_id in self._tool_panel:
section = self._tool_panel[entry.panel_section_id]
if isinstance(section, ToolSection):
self._tool_panel.record_section_for_tool_id(tool_id, entry.panel_section_id, section.name or "")
# Debug: check for mismatches
index_ids = set(self._tool_index.entries.keys()) if self._tool_index else set()
map_ids = set(self._tool_section_map.keys()) if hasattr(self, "_tool_section_map") else set()
matched = index_ids & map_ids
log.info(
f"Section map has {len(map_ids)} entries, index has {len(index_ids)} entries, {len(matched)} matched, {section_updates} updated"
)
if map_ids and index_ids:
# Show sample IDs from each for comparison
log.info(f" Sample index IDs: {list(index_ids)[:3]}")
log.info(f" Sample map IDs: {list(map_ids)[:3]}")
# Mirror ``_tool_panel``'s section/label structure into
# ``_integrated_tool_panel`` so static panel views can resolve
# sections via ``closest_section`` (which searches the integrated
# panel). Tools stay deferred — the lazy panel materialises each
# section's elems on first access. Without this seed, static views
# for the test-tool sections (e.g. ``test``, ``filter``,
# ``test_section_multi``) fail to find any section and render
# empty.
for key, value in list(self._tool_panel.items()):
if key in self._integrated_tool_panel:
continue
if isinstance(value, ToolSection):
self._integrated_tool_panel[key] = ToolSection(
{"id": value.id, "name": value.name, "version": value.version or ""}
)
else:
# Labels and other non-tool elements copy by reference.
self._integrated_tool_panel[key] = value
# === Override get_tool for lazy loading ===
@overload
@@ -1548,104 +1453,3 @@ class LazyToolBox(ToolBox):
log.debug("LazyToolBox.to_dict: returning %d tools (in_panel=False)", len(rval))
return rval
def to_panel_view(self, trans, view="default_panel_view", **kwds) -> dict[str, dict]:
"""Render a panel view's API response.
For the default view we build the response straight from
``_tool_index.entries`` — no Tool instantiation, no per-tool
re-parse. Going through the parent's ``tool_panel_contents`` ->
``apply_view`` -> ``get_tool_to_dict`` path would lazy-load every
indexed tool at request time, which is exactly what defeated
startup in the prior commit (``test_job_recovery::test_recovery``
spent ~14 minutes lazy-loading 500+ tools per restart and never
reached "ready").
Static panel views (configured via ``panel_views`` /
``panel_views_dir``) are scoped to a specific small set of
``<tool>`` entries; for those we let the parent walk
``apply_view`` so its ``ToolBoxRegistry.get_tool`` lazy-loads
only the requested few.
"""
resolved_view = view
if resolved_view == "default_panel_view":
resolved_view = self._default_panel_view(trans)
view_def = (self._tool_panel_views or {}).get(resolved_view) if hasattr(self, "_tool_panel_views") else None
# The default panel view is whatever ``AbstractToolBox.__init__`` (or our
# legacy override) registered for the "full tool panel" slot — it
# isn't an Edam view and isn't a configured StaticToolPanelView.
# ``DefaultToolPanelView`` is a *locally-defined* class inside
# ``AbstractToolBox.__init__`` now that we call ``super().__init__()``,
# so we can't ``isinstance``-check it; negate the two known
# specialisations instead.
from galaxy.tool_util.toolbox.views.static import StaticToolPanelView
is_default_view = view_def is not None and not isinstance(view_def, (EdamToolPanelView, StaticToolPanelView))
if view_def is None or is_default_view:
# Default view — render from index entries cheaply.
view_contents: dict[str, dict] = {}
sections: dict[str, dict[str, Any]] = {}
uncategorized: list[dict[str, Any]] = []
if self._tool_index is None:
return {}
include_hidden = bool(kwds.get("include_hidden", False))
for entry in self._tool_index.entries.values():
if entry.hidden and not include_hidden:
continue
tool_dict = self._index_entry_to_api_dict(entry)
section_id = entry.panel_section_id
if section_id:
section = sections.get(section_id)
if section is None:
# Mirror ``ToolSection.to_dict(only_ids=True)`` which
# the eager toolbox calls for the default view: a
# ``"tools"`` key with the list of tool ids (not
# ``"elems"`` with full dicts). Tests like
# ``test_tools::test_index`` walk
# ``tool_or_section["tools"]`` to flatten sections;
# an ``"elems"`` payload makes ``upload1`` (a
# sectioned tool) invisible to the flatten loop.
section = {
"id": section_id,
"name": entry.panel_section_name or section_id,
"model_class": "ToolSection",
"tools": [],
}
sections[section_id] = section
section["tools"].append(tool_dict["id"])
else:
uncategorized.append(tool_dict)
for section_id, section_dict in sections.items():
view_contents[section_id] = section_dict
for tool_dict in uncategorized:
view_contents[tool_dict["id"]] = tool_dict
return view_contents
# Non-default view (EDAM, static): defer to the parent. With the
# eager pipeline filling ``_integrated_tool_panel`` with ``LazyTool``
# stubs, EDAM's ``walk_loaded_tools`` reads ``tool.edam_operations``
# / ``edam_topics`` from the entry surface (no parse) and
# ``_load_tool_panel_views`` (run by ``super().__init__``) has
# already cached the rendered panel in ``_tool_panel_view_rendered``.
return super().to_panel_view(trans, view=view, **kwds)
def _index_entry_to_api_dict(self, entry: ToolIndexEntry) -> dict[str, Any]:
"""Convert an index entry to the format expected by /api/tools."""
return {
"id": entry.id,
"name": entry.name,
"version": entry.version,
"description": entry.description,
"labels": entry.labels if entry.labels else [],
"edam_operations": entry.edam_operations if entry.edam_operations else [],
"edam_topics": entry.edam_topics if entry.edam_topics else [],
"hidden": entry.hidden,
"model_class": "Tool",
"panel_section_id": entry.panel_section_id,
"panel_section_name": entry.panel_section_name,
# Minimal fields that indicate this is from index
"link": f"/api/tools/{entry.id}",
"min_width": -1,
"target": "galaxy_main",
}
test/integration/test_tool_source_storage.py
+13 -8
View File
@@ -242,11 +242,13 @@ class TestLazyToolBoxApi(BaseToolSourceStorageIntegrationTestCase):
# --- Default-panel response shape ---------------------------------------
def test_default_panel_view_section_tools_use_id_list(self):
# Pins the section-shape fix that ``ToolSection.to_dict(only_ids=True)``
# emits: each section dict has a ``tools`` key holding a list of
# tool-id strings (not full Tool dicts under ``elems``). Regression
# for ``test_tools::test_index`` which walks ``tool_or_section["tools"]``
# to flatten sections; a different shape makes upload1 invisible.
# Pins ``ToolSection.to_dict(only_ids=True)``: each section dict has a
# ``tools`` key holding tool-id strings (interleaved with
# ``ToolSectionLabel`` dicts where the conf places labels). The
# regression this guards against is *Tool* dicts appearing in
# ``tools`` — that breaks ``test_tools::test_index``, which walks
# ``tool_or_section["tools"]`` to flatten sections and would
# otherwise miss sectioned tools like ``upload1``.
response = self._get("tool_panels/default")
self._assert_status_code_is(response, 200)
panel = response.json()
@@ -256,9 +258,12 @@ class TestLazyToolBoxApi(BaseToolSourceStorageIntegrationTestCase):
if isinstance(entry, dict) and entry.get("model_class") == "ToolSection":
sections_seen += 1
assert "tools" in entry, f"section {entry_id} missing 'tools' key"
assert all(
isinstance(t, str) for t in entry["tools"]
), f"section {entry_id} should hold tool ids as strings, got {entry['tools'][:3]}"
for item in entry["tools"]:
if isinstance(item, str):
continue
assert (
isinstance(item, dict) and item.get("model_class") == "ToolSectionLabel"
), f"section {entry_id} should hold tool-id strings or ToolSectionLabel dicts, got {item!r}"
# Sanity: the framework conf has at least one section, otherwise the
# assertion above never ran.
assert sections_seen > 0, "expected at least one section in default panel view"