Skip to content

Commit bce63eb

Browse files
committed
LazyToolBox: defer panel-view rendering + sync removals to the index
Two more Integration shard 2 fallouts after 38da771: 1. ``_load_tool_panel_views`` ran inside ``_init_lazy_toolbox`` — before ``_load_index_from_store`` populated the index. Static panel views call ``toolbox_registry.has_tool(tool_id)`` for every entry in ``apply_view``; with an empty index the check failed and dropped every tool, surfacing as ``Failed to find tool_id <id> from parent toolbox, cannot load into panel view`` warnings and a custom-view test (``test_panel_views::test_custom_label_order``) that received sections instead of the expected flat list. Move the ``_load_tool_panel_views`` call to *after* ``_populate_tool_registry_from_index`` so the static views render against the populated registry. 2. ``AbstractToolBox.remove_tool_by_id`` only deletes from ``_tools_by_id``. The lazy path keeps a parallel ``_tool_index`` that ``LazyToolBox.get_tool`` reads first, so a removal didn't actually remove anything — the next call lazy-loaded the same tool straight back. Tests ``test_credentials::test_list_credentials_with_missing_tool`` and ``test_fail_job_tool_unavailable::test_fail_job_when_tool_unavailable`` relied on a real removal. Override ``remove_tool_by_id`` to force-materialise (so the parent's bookkeeping has a real Tool to work against), call super, then drop the id from ``_tool_index.entries`` / ``entries_by_version`` and prune matching entries from the LRU cache. Integration test ``test_tool_source_storage.py`` still passes (5/5).
1 parent 38da771 commit bce63eb

1 file changed

Lines changed: 40 additions & 3 deletions

File tree

lib/galaxy/tools/lazy_toolbox.py

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,14 @@ def __init__(
206206
# This allows has_tool() and similar checks to work without loading
207207
self._populate_tool_registry_from_index()
208208

209+
# Render static panel views now that the index is populated. Before
210+
# this point ``apply_view`` would have ``has_tool`` return False for
211+
# every indexed-but-unloaded tool and the resulting view would be
212+
# missing every entry. See the deferred-render comment in
213+
# ``_init_lazy_toolbox``.
214+
if self.app.name == "galaxy":
215+
self._load_tool_panel_views()
216+
209217
log.info(f"LazyToolBox initialized with {len(self._tools_by_id)} tools (cache_size={cache_size})")
210218
self._warn_if_index_misses_panel_tools()
211219

@@ -300,9 +308,14 @@ def _init_lazy_toolbox(
300308
# but don't load the actual tools
301309
self._init_panel_structure_from_configs(config_filenames)
302310

303-
# Load tool panel views (required for panel_has_tool checks)
304-
if self.app.name == "galaxy":
305-
self._load_tool_panel_views()
311+
# ``_load_tool_panel_views`` materialises every static panel view by
312+
# walking ``apply_view``, which calls ``toolbox_registry.has_tool``.
313+
# In the lazy path the index isn't populated until ``_load_index_from_store``
314+
# runs (right after this method returns), so calling it here would
315+
# see an empty toolbox and drop every tool from the static views with
316+
# a "Failed to find tool_id ... cannot load into panel view" warning.
317+
# Defer the rendering until after the index is loaded — see the
318+
# follow-up call in ``__init__``.
306319

307320
if save_integrated_tool_panel:
308321
self._save_integrated_tool_panel()
@@ -1344,6 +1357,30 @@ def _register_loaded_tool(self, tool: "Tool") -> None:
13441357
if entry and entry.hidden:
13451358
tool.hidden = True
13461359

1360+
def remove_tool_by_id(self, tool_id: str, remove_from_panel: bool = True):
1361+
"""Also drop the tool from the lazy index + LRU cache.
1362+
1363+
``AbstractToolBox.remove_tool_by_id`` only deletes from
1364+
``_tools_by_id``. In the lazy path that's not enough — ``get_tool``
1365+
re-loads the tool from ``_tool_index`` on the next request, so the
1366+
tool effectively comes back. The eager toolbox doesn't have this
1367+
problem because the tool object isn't created from a serialised
1368+
store. Mirror the deletion across the two backing stores.
1369+
"""
1370+
# Force-materialise so the eager parent's bookkeeping (``_tools_by_old_id``,
1371+
# panel removal, lineage, tool cache expiry) gets a real Tool to work
1372+
# against.
1373+
if self._tools_by_id.get(tool_id) is None:
1374+
self.get_tool(tool_id=tool_id)
1375+
result = super().remove_tool_by_id(tool_id, remove_from_panel=remove_from_panel)
1376+
if self._tool_index is not None:
1377+
self._tool_index.entries.pop(tool_id, None)
1378+
self._tool_index.entries_by_version.pop(tool_id, None)
1379+
with self._cache_lock:
1380+
for key in [k for k in self._tool_object_cache.keys() if k.startswith(f"{tool_id}:")]:
1381+
self._tool_object_cache.pop(key, None)
1382+
return result
1383+
13471384
@property
13481385
def tools_by_id(self) -> "_LazyToolsByIdView":
13491386
"""Lazy-loading view over ``_tools_by_id``.

0 commit comments

Comments
 (0)