Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assert that data_column parameters have a valid data_ref #18949

Merged
merged 3 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/galaxy/tool_shed/tools/tool_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def load_tool_from_config(self, repository_id, full_path):
tool = None
valid = False
error_message = (
f'This file requires an entry for "{str(e)}" in the tool_data_table_conf.xml file. Upload a file '
f'This file requires an entry for "{str(e)}" in the tool_data_table_conf.xml file. Upload a file '
)
error_message += (
"named tool_data_table_conf.xml.sample to the repository that includes the required entry to correct "
Expand Down
4 changes: 4 additions & 0 deletions lib/galaxy/tool_util/xsd/galaxy.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ List of behavior changes associated with profile versions:
- Do not use Galaxy python environment for `data_source_async` tools.
- Drop request parameters received by data source tools that are not declared in `<request_param_translation>` section.
### 24.2
- require a valid `data_ref` attribute for `data_column` parameters
### Examples
A normal tool:
Expand Down
5 changes: 3 additions & 2 deletions lib/galaxy/tools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1665,8 +1665,9 @@ def parse_param_elem(self, input_source: InputSource, enctypes, context) -> Tool
# form when it changes
for name in param.get_dependencies():
# Let it throw exception, but give some hint what the problem might be
if name not in context:
log.error(f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'")
assert (
name in context
), f"Tool with id '{self.id}': Could not find dependency '{name}' of parameter '{param.name}'"
context[name].refresh_on_change = True
return param

Expand Down
4 changes: 4 additions & 0 deletions lib/galaxy/tools/parameters/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,10 @@ def __init__(self, tool, input_source):
if self.accept_default:
self.optional = True
self.data_ref = input_source.get("data_ref", None)
profile = tool.profile if tool else None
assert (
profile is None or Version(str(profile)) < Version("24.2") or self.data_ref is not None
), f"data_column parameter {self.name} requires a valid data_ref attribute"
self.ref_input = None
# Legacy style default value specification...
self.default_value = input_source.get("default_value", None)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<tool id="missing_data_ref" name="missing_data_ref" version="1.0" profile="23.0">
<requirements>
</requirements>
<command detect_errors="aggressive">
<![CDATA[
echo $column
]]>
</command>
<inputs>
<param name="input" type="data" format="tabular"/>
<param name="column" type="data_column" />
</inputs>
<outputs>
<data name="output" format="text"/>
</outputs>
<tests>
</tests>
<help/>
</tool>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<tool id="wrong_data_ref" name="wrong_data_ref" version="1.0" profile="23.0">
<requirements>
</requirements>
<command detect_errors="aggressive">
<![CDATA[
echo $column
]]>
</command>
<inputs>
<param name="input" type="data" format="tabular"/>
<param name="column" type="data_column" data_ref="nonsense" />
</inputs>
<outputs>
<data name="output" format="text"/>
</outputs>
<tests>
</tests>
<help/>
</tool>
18 changes: 18 additions & 0 deletions test/unit/tool_shed/test_tool_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
BOWTIE2_INDICES = os.path.join(
galaxy_directory(), "lib/tool_shed/test/test_data/bowtie2_loc_sample/bowtie2_indices.loc.sample"
)
MISSING_DATA_REF_DIR = os.path.join(galaxy_directory(), "lib/tool_shed/test/test_data/repos/missing_data_ref")
WRONG_DATA_REF_DIR = os.path.join(galaxy_directory(), "lib/tool_shed/test/test_data/repos/wrong_data_ref")


def test_validate_valid_tool():
Expand Down Expand Up @@ -64,6 +66,22 @@ def test_validate_tool_without_index():
assert not tool.params_with_missing_index_file


def test_validate_missing_data_ref():
repo_dir = MISSING_DATA_REF_DIR
with get_tool_validator() as tv:
full_path = os.path.join(repo_dir, "missing_data_ref.xml")
tool, valid, message = tv.load_tool_from_config(repository_id=None, full_path=full_path)
assert valid is False


def test_validate_wrong_data_ref():
repo_dir = WRONG_DATA_REF_DIR
with get_tool_validator() as tv:
full_path = os.path.join(repo_dir, "wrong_data_ref.xml")
tool, valid, message = tv.load_tool_from_config(repository_id=None, full_path=full_path)
assert valid is False


@contextmanager
def get_tool_validator():
app = MockApp()
Expand Down
Loading