mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Drop DataTableFileConflict; two tables may share a loc file
Galaxy ships picard_indexes and srma_indexes pointing at the same loc file, so the cross-table file-path check raised on every startup. Keep only the same-name column-mismatch protection and update the production-shed test to expect no per-revision tool_data_table_conf.xml for non-DM repos.
This commit is contained in:
@@ -8,10 +8,7 @@ from typing import (
|
||||
|
||||
from galaxy.tool_shed.galaxy_install.client import InstallationTarget
|
||||
from galaxy.tool_shed.util import hg_util
|
||||
from galaxy.tool_util.data import (
|
||||
DataTableColumnMismatch,
|
||||
DataTableFileConflict,
|
||||
)
|
||||
from galaxy.tool_util.data import DataTableColumnMismatch
|
||||
from galaxy.util import (
|
||||
Element,
|
||||
SubElement,
|
||||
@@ -219,16 +216,14 @@ class ShedToolDataTableManager(BaseShedToolDataTableManager):
|
||||
if elem.tag != "table":
|
||||
kept_elems.append(elem)
|
||||
continue
|
||||
candidate_file_paths: list[str] = []
|
||||
for file_elem in elem.findall("file"):
|
||||
path = file_elem.get("path", None)
|
||||
if path:
|
||||
new_path = os.path.normpath(os.path.join(shared_loc_dir, os.path.split(path)[1]))
|
||||
file_elem.set("path", new_path)
|
||||
candidate_file_paths.append(new_path)
|
||||
table_name = elem.get("name") or ""
|
||||
incoming_columns = _parse_table_columns(elem)
|
||||
self.app.tool_data_tables.assert_data_table_consistency(table_name, incoming_columns, candidate_file_paths)
|
||||
self.app.tool_data_tables.assert_data_table_consistency(table_name, incoming_columns)
|
||||
existing = registered_tables.get(table_name) if table_name else None
|
||||
if existing is not None and getattr(existing, "columns", None) is not None:
|
||||
# Already registered with matching columns; skip to avoid duplicate <table> entry.
|
||||
@@ -251,7 +246,6 @@ ToolDataTableManager = ShedToolDataTableManager
|
||||
|
||||
__all__ = (
|
||||
"DataTableColumnMismatch",
|
||||
"DataTableFileConflict",
|
||||
"ToolDataTableManager",
|
||||
"ShedToolDataTableManager",
|
||||
)
|
||||
|
||||
@@ -23,7 +23,6 @@ from typing import (
|
||||
BinaryIO,
|
||||
Callable,
|
||||
Dict,
|
||||
Iterable,
|
||||
List,
|
||||
Optional,
|
||||
overload,
|
||||
@@ -154,28 +153,6 @@ class DataTableColumnMismatch(Exception):
|
||||
)
|
||||
|
||||
|
||||
class DataTableFileConflict(Exception):
|
||||
"""Two data tables with different names reference the same loc file."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
path: str,
|
||||
candidate_name: str,
|
||||
candidate_columns: Dict[str, int],
|
||||
existing_name: str,
|
||||
existing_columns: Dict[str, int],
|
||||
):
|
||||
self.path = path
|
||||
self.candidate_name = candidate_name
|
||||
self.candidate_columns = candidate_columns
|
||||
self.existing_name = existing_name
|
||||
self.existing_columns = existing_columns
|
||||
super().__init__(
|
||||
f"Data table {candidate_name!r} declares loc file {path!r}, but that file is already "
|
||||
f"registered to data table {existing_name!r}."
|
||||
)
|
||||
|
||||
|
||||
class ToolDataTable(Dictifiable):
|
||||
type_key: str
|
||||
data: List[List[str]]
|
||||
@@ -1022,31 +999,13 @@ class ToolDataTableManager(Dictifiable):
|
||||
self,
|
||||
candidate_name: str,
|
||||
candidate_columns: Dict[str, int],
|
||||
candidate_file_paths: Iterable[str],
|
||||
) -> None:
|
||||
"""
|
||||
Raise if registering ``candidate_name`` would conflict with current state:
|
||||
an existing table with the same name but different columns, or any
|
||||
``candidate_file_paths`` already owned by a different table name.
|
||||
"""
|
||||
"""Raise if ``candidate_name`` is already registered with different columns."""
|
||||
existing = self.data_tables.get(candidate_name)
|
||||
if existing is not None:
|
||||
existing_columns = getattr(existing, "columns", None)
|
||||
if existing_columns is not None and existing_columns != candidate_columns:
|
||||
raise DataTableColumnMismatch(candidate_name, existing_columns, candidate_columns)
|
||||
candidate_realpaths = {os.path.realpath(p) for p in candidate_file_paths if p}
|
||||
if not candidate_realpaths:
|
||||
return
|
||||
for other_name, other_table in self.data_tables.items():
|
||||
if other_name == candidate_name:
|
||||
continue
|
||||
other_filenames = getattr(other_table, "filenames", None) or {}
|
||||
for other_path in other_filenames:
|
||||
if os.path.realpath(other_path) in candidate_realpaths:
|
||||
other_columns = getattr(other_table, "columns", None) or {}
|
||||
raise DataTableFileConflict(
|
||||
other_path, candidate_name, candidate_columns, other_name, other_columns
|
||||
)
|
||||
|
||||
def to_dict(
|
||||
self, view: str = "collection", value_mapper: Optional[Dict[str, Callable]] = None
|
||||
@@ -1085,11 +1044,6 @@ class ToolDataTableManager(Dictifiable):
|
||||
other_config_dict=self.other_config_dict,
|
||||
)
|
||||
table_elems.append(table_elem)
|
||||
self.assert_data_table_consistency(
|
||||
table.name,
|
||||
getattr(table, "columns", {}) or {},
|
||||
getattr(table, "filenames", {}) or {},
|
||||
)
|
||||
if table.name not in self.data_tables:
|
||||
self.data_tables[table.name] = table
|
||||
log.debug("Loaded tool data table '%s' from file '%s'", table.name, config_filename)
|
||||
|
||||
@@ -43,6 +43,7 @@ def test_against_production_shed(tmp_path: Path):
|
||||
assert tool_guid in f.read()
|
||||
repo_path = tmp_path / "tools" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision
|
||||
assert repo_path.exists()
|
||||
# featurecounts is not a Data Manager — install must not register a per-revision data table config.
|
||||
tool_data_table_path = (
|
||||
tmp_path
|
||||
/ "tool_data"
|
||||
@@ -53,7 +54,7 @@ def test_against_production_shed(tmp_path: Path):
|
||||
/ repo_revision
|
||||
/ "tool_data_table_conf.xml"
|
||||
)
|
||||
assert tool_data_table_path.exists()
|
||||
assert not tool_data_table_path.exists()
|
||||
|
||||
install_model_context = cast("install_model_scoped_session", install_target.install_model.session)
|
||||
query = install_model_context.query(ToolShedRepository).where(ToolShedRepository.name == repo_name)
|
||||
|
||||
@@ -7,7 +7,6 @@ import pytest
|
||||
|
||||
from galaxy.tool_shed.tools.data_table_manager import (
|
||||
DataTableColumnMismatch,
|
||||
DataTableFileConflict,
|
||||
ShedToolDataTableManager,
|
||||
)
|
||||
from galaxy.tool_util.data import ToolDataTableManager
|
||||
@@ -164,23 +163,6 @@ def test_column_match_with_column_elements_dedupes(tmp_path):
|
||||
assert kept_elems == []
|
||||
|
||||
|
||||
def test_file_path_conflict_raises(tmp_path):
|
||||
stdtm, repo, samples, captured, tool_data_path, _, _ = _make_stdtm(tmp_path)
|
||||
shared_loc = os.path.join(tool_data_path, "all_fasta.loc")
|
||||
stdtm.app.tool_data_tables.data_tables = {
|
||||
"other_table": _registered_table(
|
||||
{"value": 0, "name": 1},
|
||||
filenames={shared_loc: {"found": True}},
|
||||
),
|
||||
}
|
||||
|
||||
with pytest.raises(DataTableFileConflict) as exc_info:
|
||||
stdtm.install_tool_data_tables(repo, samples)
|
||||
assert exc_info.value.candidate_name == "all_fasta"
|
||||
assert exc_info.value.existing_name == "other_table"
|
||||
assert not captured["to_xml_calls"]
|
||||
|
||||
|
||||
def test_parse_table_columns_aliases_name_to_value():
|
||||
from galaxy.tool_shed.tools.data_table_manager import _parse_table_columns
|
||||
|
||||
|
||||
@@ -1,9 +1,6 @@
|
||||
import pytest
|
||||
|
||||
from galaxy.tool_util.data import (
|
||||
DataTableColumnMismatch,
|
||||
DataTableFileConflict,
|
||||
)
|
||||
from galaxy.tool_util.data import DataTableColumnMismatch
|
||||
|
||||
LOC_ALPHA_CONTENTS_V2 = """
|
||||
data1 data1name ${__HERE__}/data1/entry.txt
|
||||
@@ -11,14 +8,6 @@ data2 data2name ${__HERE__}/data2/entry.txt
|
||||
data3 data3name ${__HERE__}/data3/entry.txt
|
||||
"""
|
||||
|
||||
CONFLICTING_TABLE_CONF_XML = """<tables>
|
||||
<table name="other_table" comment_char="#">
|
||||
<columns>value, name, path</columns>
|
||||
<file path="{loc_path}" />
|
||||
</table>
|
||||
</tables>
|
||||
"""
|
||||
|
||||
COLUMN_DIVERGENT_TABLE_CONF_XML = """<tables>
|
||||
<table name="testalpha" comment_char="#">
|
||||
<columns>value, name, path, extra</columns>
|
||||
@@ -85,54 +74,22 @@ def test_to_json(merged_tdt_manager, tmp_path):
|
||||
assert json_path.exists()
|
||||
|
||||
|
||||
def test_assert_data_table_consistency_accepts_new_table(tdt_manager, tmp_path):
|
||||
def test_assert_data_table_consistency_accepts_new_table(tdt_manager):
|
||||
tdt_manager.assert_data_table_consistency(
|
||||
"brand_new_table",
|
||||
{"value": 0, "name": 1, "path": 2},
|
||||
[str(tmp_path / "brand_new_table.loc")],
|
||||
)
|
||||
|
||||
|
||||
def test_assert_data_table_consistency_accepts_matching_redefinition(tdt_manager, tmp_path):
|
||||
def test_assert_data_table_consistency_accepts_matching_redefinition(tdt_manager):
|
||||
existing = tdt_manager["testalpha"]
|
||||
tdt_manager.assert_data_table_consistency(
|
||||
"testalpha",
|
||||
existing.columns,
|
||||
list(existing.filenames),
|
||||
)
|
||||
tdt_manager.assert_data_table_consistency("testalpha", existing.columns)
|
||||
|
||||
|
||||
def test_assert_data_table_consistency_raises_column_mismatch(tdt_manager, tmp_path):
|
||||
def test_assert_data_table_consistency_raises_column_mismatch(tdt_manager):
|
||||
with pytest.raises(DataTableColumnMismatch) as exc_info:
|
||||
tdt_manager.assert_data_table_consistency(
|
||||
"testalpha",
|
||||
{"value": 0, "name": 1, "path": 2, "extra": 3},
|
||||
[str(tmp_path / "testalpha.loc")],
|
||||
)
|
||||
assert exc_info.value.table_name == "testalpha"
|
||||
|
||||
|
||||
def test_assert_data_table_consistency_raises_file_conflict(tdt_manager, tmp_path):
|
||||
shared_loc = str(tmp_path / "testalpha.loc")
|
||||
with pytest.raises(DataTableFileConflict) as exc_info:
|
||||
tdt_manager.assert_data_table_consistency(
|
||||
"other_table",
|
||||
{"value": 0, "name": 1, "path": 2},
|
||||
[shared_loc],
|
||||
)
|
||||
assert exc_info.value.candidate_name == "other_table"
|
||||
assert exc_info.value.existing_name == "testalpha"
|
||||
|
||||
|
||||
def test_load_from_config_file_raises_on_file_path_conflict(tdt_manager, tmp_path):
|
||||
conflicting_conf = tmp_path / "conflict.xml"
|
||||
conflicting_conf.write_text(CONFLICTING_TABLE_CONF_XML.format(loc_path=str(tmp_path / "testalpha.loc")))
|
||||
with pytest.raises(DataTableFileConflict):
|
||||
tdt_manager.load_from_config_file(str(conflicting_conf), str(tmp_path), from_shed_config=True)
|
||||
|
||||
|
||||
def test_load_from_config_file_raises_on_column_mismatch_same_name(tdt_manager, tmp_path):
|
||||
column_conflict_conf = tmp_path / "column_conflict.xml"
|
||||
column_conflict_conf.write_text(COLUMN_DIVERGENT_TABLE_CONF_XML.format(loc_path=str(tmp_path / "testalpha.loc")))
|
||||
with pytest.raises(DataTableColumnMismatch):
|
||||
tdt_manager.load_from_config_file(str(column_conflict_conf), str(tmp_path), from_shed_config=True)
|
||||
|
||||
Reference in New Issue
Block a user