From caea1912b946731906f8e07e8a1646caed5e39bf Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 3 Jul 2026 16:48:17 +0200 Subject: [PATCH] Use plain config/attribute access; unify tool-file detection with tool_util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GalaxyAppConfiguration materializes every schema option, so the hasattr/getattr guards around tool_configs, shed_tool_config_file, migrated_tools_config, root, tool_search_index_dir and use_lazy_toolbox were dead (StandaloneInstallationTarget's Config stub gains the use_lazy_toolbox attribute the real config carries). The tool_source_url getattr had no matching schema option — the default sqlalchemy store is now disk-path only; full URLs remain available via named tool_source_stores entries. getattr(entry, "source_path") was hiding a real gap: ToolIndexEntry never carried the field, so LazyTool.config_file always materialised. The populator now stamps source_path onto every entry. discover.py's duplicated _looks_like_a_tool is replaced by the real galaxy.tool_util.loader_directory.looks_like_a_tool (threading enable_beta_tool_formats), and the works-without-galaxy fallback for MODEL_TOOLS_PATH is gone — the populator always runs with galaxy importable. --- .../tools/tool_panel_manager.py | 2 +- .../tool_shed/unittest_utils/__init__.py | 1 + lib/galaxy/tool_source_store/__init__.py | 5 +- lib/galaxy/tool_source_store/discover.py | 64 +++++-------------- lib/galaxy/tool_source_store/index.py | 3 + lib/galaxy/tool_source_store/populator.py | 7 +- lib/galaxy/tools/lazy_toolbox.py | 11 ++-- 7 files changed, 29 insertions(+), 64 deletions(-) diff --git a/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py b/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py index 8e6d14a27c3..a150a7b21da 100644 --- a/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py +++ b/lib/galaxy/tool_shed/galaxy_install/tools/tool_panel_manager.py @@ -140,7 +140,7 @@ class ToolPanelManager: ) if new_install: tool_path = shed_tool_conf_dict["tool_path"] - use_lazy_toolbox = getattr(self.app.config, "use_lazy_toolbox", False) + use_lazy_toolbox = self.app.config.use_lazy_toolbox if use_lazy_toolbox: # The populator writes ``StoredToolSource`` + ``ToolIndexEntry`` # + whoosh for every new tool file, then broadcasts diff --git a/lib/galaxy/tool_shed/unittest_utils/__init__.py b/lib/galaxy/tool_shed/unittest_utils/__init__.py index 5dd8ec0fbbf..42d8dd0b024 100644 --- a/lib/galaxy/tool_shed/unittest_utils/__init__.py +++ b/lib/galaxy/tool_shed/unittest_utils/__init__.py @@ -74,6 +74,7 @@ class Config: shed_tools_dir: str edam_panel_views: list = [] tool_configs: list = [] + use_lazy_toolbox: bool = False shed_tool_data_table_config: str shed_data_manager_config_file: str diff --git a/lib/galaxy/tool_source_store/__init__.py b/lib/galaxy/tool_source_store/__init__.py index 4a82f438cc2..b3bb29c8667 100644 --- a/lib/galaxy/tool_source_store/__init__.py +++ b/lib/galaxy/tool_source_store/__init__.py @@ -230,13 +230,10 @@ def _build_default_store( if backend in ("sqlalchemy", "sqlite"): from .sqlalchemy import SqlAlchemyToolSourceStore - url = getattr(config, "tool_source_url", None) path = config.tool_source_disk_path - if url: - return SqlAlchemyToolSourceStore(url=url, read_only=False) if path: return SqlAlchemyToolSourceStore(path=path, read_only=False) - raise ConfigurationError(f"{backend!r} backend requires tool_source_url or tool_source_disk_path") + raise ConfigurationError(f"{backend!r} backend requires tool_source_disk_path") raise ConfigurationError(f"Unknown tool source store backend: {backend}") diff --git a/lib/galaxy/tool_source_store/discover.py b/lib/galaxy/tool_source_store/discover.py index 9cf610e5aae..8d3bc887460 100644 --- a/lib/galaxy/tool_source_store/discover.py +++ b/lib/galaxy/tool_source_store/discover.py @@ -24,6 +24,7 @@ from typing import ( TYPE_CHECKING, ) +from galaxy.tool_util.loader_directory import looks_like_a_tool from galaxy.tool_util.toolbox.parser import ( get_toolbox_parser, ToolConfItem, @@ -75,16 +76,16 @@ def get_tool_configs(config: "GalaxyAppConfiguration") -> list[str]: configs = [] # Get main tool config files - if hasattr(config, "tool_configs") and config.tool_configs: + if config.tool_configs: configs.extend(config.tool_configs) # Ensure shed_tool_config_file is included if not already - if hasattr(config, "shed_tool_config_file") and config.shed_tool_config_file: + if config.shed_tool_config_file: if config.shed_tool_config_file not in configs: configs.append(config.shed_tool_config_file) # Include migrated_tools_config if present - if hasattr(config, "migrated_tools_config") and config.migrated_tools_config: + if config.migrated_tools_config: if config.migrated_tools_config not in configs: configs.append(config.migrated_tools_config) @@ -125,21 +126,15 @@ def _resolve_tool_path(tool_path: str | None, config_filename: str, root_dir: st return os.path.abspath(os.path.join(tool_conf_dir, tool_path)) -def _resolve_file_template_kwds(root_dir: str | None) -> dict[str, str]: +def _resolve_file_template_kwds() -> dict[str, str]: """Resolve template variables that tool conf ``file=...`` attributes may use. - Mirrors :py:meth:`galaxy.tools.ToolBox._path_template_kwds`. The galaxy - import is optional so this script-local helper still works when galaxy is - not importable; falls back to a path computed from ``root_dir``. + Mirrors :py:meth:`galaxy.tools.ToolBox._path_template_kwds`. """ - try: - # Lazy + optional: helper must still work outside a galaxy install. - from galaxy.tools import MODEL_TOOLS_PATH - except Exception: - if root_dir: - MODEL_TOOLS_PATH = os.path.abspath(os.path.join(root_dir, "lib", "galaxy", "tools")) - else: - return {} + # Local import: only a path constant is needed, and a module-level import + # would pull the whole galaxy.tools package into every discover() caller. + from galaxy.tools import MODEL_TOOLS_PATH + return {"model_tools_path": MODEL_TOOLS_PATH} @@ -163,36 +158,6 @@ def _iter_tool_items( yield from _iter_tool_items(item.items, parent_section=item) -def _looks_like_a_tool(path: str) -> bool: - """Cheap filter mirroring ``galaxy.tool_util.toolbox.base.looks_like_a_tool``. - - We only want XML or YAML/CWL files that plausibly define a tool. Avoid - importing the real ``looks_like_a_tool`` so this script-local helper still - works without galaxy on sys.path. - """ - name = os.path.basename(path) - if name.startswith((".", "_")) or "macro" in name.lower(): - return False - ext = os.path.splitext(name)[1].lower() - if ext == ".xml": - try: - with open(path, encoding="utf-8") as fh: - head = fh.read(2000) - except Exception: - return False - return " Iterator[str]: """Yield candidate tool file paths under ``directory``. @@ -218,6 +183,7 @@ def _walk_tool_dir(directory: str, recursive: bool) -> Iterator[str]: def discover_tools_from_config( config_filename: str, root_dir: str | None = None, + enable_beta_formats: bool = False, ) -> Iterator[DiscoveredTool]: """ Discover all tools from a single tool configuration file. @@ -248,7 +214,7 @@ def discover_tools_from_config( # (e.g. ```` in # tool_conf.xml.sample). Without expanding this, those tools are silently # dropped at the os.path.exists check below. - file_template_kwds = _resolve_file_template_kwds(root_dir) + file_template_kwds = _resolve_file_template_kwds() for item, section in _iter_tool_items(tool_conf_source.parse_items()): section_id = section.get("id") if section is not None else None @@ -264,7 +230,7 @@ def discover_tools_from_config( directory = os.path.join(resolved_tool_path, dir_attr) recursive = str(item.get("recursive", "true")).lower() != "false" for candidate in _walk_tool_dir(os.path.normpath(directory), recursive): - if not _looks_like_a_tool(candidate): + if not looks_like_a_tool(candidate, enable_beta_formats=enable_beta_formats): continue yield DiscoveredTool( path=candidate, @@ -326,12 +292,12 @@ def discover_tools( Yields: DiscoveredTool objects for each tool found. """ - root_dir = getattr(config, "root", None) + root_dir = config.root seen_paths: set = set() # Discover from all tool config files for config_filename in get_tool_configs(config): - for tool in discover_tools_from_config(config_filename, root_dir): + for tool in discover_tools_from_config(config_filename, root_dir, config.enable_beta_tool_formats): if tool.path not in seen_paths: seen_paths.add(tool.path) yield tool diff --git a/lib/galaxy/tool_source_store/index.py b/lib/galaxy/tool_source_store/index.py index 3877d86de9e..6247416b543 100644 --- a/lib/galaxy/tool_source_store/index.py +++ b/lib/galaxy/tool_source_store/index.py @@ -46,6 +46,7 @@ class ToolIndexEntry: # === Source Reference === source_hash: str = "" source_class: str = "XmlToolSource" + source_path: str | None = None # === Status === hidden: bool = False @@ -142,6 +143,7 @@ class ToolIndexEntry: "edam_topics": self.edam_topics, "source_hash": self.source_hash, "source_class": self.source_class, + "source_path": self.source_path, "hidden": self.hidden, "disabled": self.disabled, "require_login": self.require_login, @@ -179,6 +181,7 @@ class ToolIndexEntry: edam_topics=data.get("edam_topics", []), source_hash=data.get("source_hash", ""), source_class=data.get("source_class", "XmlToolSource"), + source_path=data.get("source_path"), hidden=data.get("hidden", False), disabled=data.get("disabled", False), require_login=data.get("require_login", False), diff --git a/lib/galaxy/tool_source_store/populator.py b/lib/galaxy/tool_source_store/populator.py index c93ef100779..1d93484adc3 100644 --- a/lib/galaxy/tool_source_store/populator.py +++ b/lib/galaxy/tool_source_store/populator.py @@ -360,7 +360,7 @@ def _build_whoosh_for_store(config, store_name: str, tool_index) -> None: populator's job is to write what it can; an unbuildable index is a deploy issue, not a populator-CLI fatal). """ - index_dir = whoosh_dir_for_store(getattr(config, "tool_search_index_dir", None), store_name) + index_dir = whoosh_dir_for_store(config.tool_search_index_dir, store_name) if index_dir is None: return try: @@ -465,6 +465,7 @@ def build_index_entry_from_source( edam_topics=edam_topics, source_hash=stored.hash, source_class=stored.tool_source_class, + source_path=stored.source_path, hidden=hidden, require_login=require_login, tool_type=tool_type, @@ -474,8 +475,8 @@ def build_index_entry_from_source( except Exception as e: log.warning( "Error building index entry (id=%s, hash=%s): %s", - getattr(stored, "tool_id", None), - getattr(stored, "hash", None), + stored.tool_id, + stored.hash, e, ) return None diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index f4338749c12..e0878632302 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -212,13 +212,10 @@ class LazyTool: @property def config_file(self) -> str | None: - # ``StoredToolSource.source_path`` was added on this branch precisely - # so the lazy path can resolve a tool back to its on-disk conf without - # parsing. Older entries serialised before that field exists return - # ``None`` — callers that need a real path (``_write_integrated_tool_panel_config_file``, - # ``get_externally_referenced_paths``) fall through to ``__getattr__`` - # and materialise. - return getattr(self._entry, "source_path", None) + # ``source_path`` is stamped by the populator; entries serialized + # before the field existed deserialize as ``None`` and callers that + # need a real path fall through to ``__getattr__`` and materialise. + return self._entry.source_path @property def lineage(self):