Skip to content

Commit a6ce0d6

Browse files
committed
Relativize bundle paths from the known job output root
Data managers can record absolute paths below $out_file.extra_files_path, while bundle writing runs on the job handler after that directory has been collected into the object store. Relativizing against dataset.extra_files_path then escapes with '..' and leaves the compute-side path in the bundle index. The job working directory is already known. For outputs_to_working_directory, reconstruct the exact compute-side extra-files root with the same uuid-keyed output-path rule used during command construction, then pass that per-output root to bundle writing. For other outputs, retain the dataset extra-files path. This restores ordinary os.path.relpath containment checks, including '.' for the extra-files root itself, instead of inferring the root from a matching directory component. It handles split job/object-store prefixes and id-keyed object stores without changing the bundle format or sending anything to the compute node. Regression tests cover the split-prefix case, uuid-keyed compute paths on id-keyed stores, the root directory, and nested dataset-like directory names.
1 parent e83fb88 commit a6ce0d6

6 files changed

Lines changed: 192 additions & 9 deletions

File tree

lib/galaxy/tool_shed/galaxy_install/client.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import threading
2+
from collections.abc import Mapping
23
from typing import (
34
Any,
45
runtime_checkable,
@@ -27,7 +28,9 @@ class DataManagerInterface(Protocol):
2728

2829
def process_result(self, out_data): ...
2930

30-
def write_bundle(self, out: dict[str, OutputDataset]) -> dict[str, OutputDataset]: ...
31+
def write_bundle(
32+
self, out: dict[str, OutputDataset], source_extra_files_paths: Mapping[str, str] | None = None
33+
) -> dict[str, OutputDataset]: ...
3134

3235

3336
class DataManagersInterface(Protocol):

lib/galaxy/tool_shed/unittest_utils/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class DummyDataManager(DataManagerInterface):
144144
def process_result(self, out_data):
145145
return None
146146

147-
def write_bundle(self, out) -> dict[str, OutputDataset]:
147+
def write_bundle(self, out, source_extra_files_paths=None) -> dict[str, OutputDataset]:
148148
return {}
149149

150150

lib/galaxy/tool_util/data/__init__.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from collections.abc import (
1818
Callable,
1919
Iterator,
20+
Mapping,
2021
)
2122
from contextlib import contextmanager
2223
from dataclasses import dataclass
@@ -1341,6 +1342,7 @@ def write_bundle(
13411342
out_data: dict[str, OutputDataset],
13421343
bundle_description: DataTableBundleProcessorDescription,
13431344
repo_info: RepoInfo | None,
1345+
source_extra_files_paths: Mapping[str, str] | None = None,
13441346
) -> dict[str, OutputDataset]:
13451347
"""Writes bundle and returns bundle path."""
13461348
data_manager_dict = _data_manager_dict(out_data, ensure_single_output=True)
@@ -1350,8 +1352,9 @@ def write_bundle(
13501352
continue
13511353

13521354
extra_files_path = dataset.extra_files_path
1355+
source_extra_files_path = (source_extra_files_paths or {}).get(output_name, extra_files_path)
13531356
_relativize_bundle_data_table_paths(
1354-
data_manager_dict.get("data_tables", {}), extra_files_path, bundle_description
1357+
data_manager_dict.get("data_tables", {}), source_extra_files_path, bundle_description
13551358
)
13561359
bundle = DataTableBundle(
13571360
data_tables=data_manager_dict.get("data_tables", {}),
@@ -1398,13 +1401,13 @@ def _relativize_bundle_data_table_paths(
13981401
extra_files_path: str,
13991402
bundle_description: DataTableBundleProcessorDescription,
14001403
) -> None:
1401-
"""Rewrite absolute path values under ``extra_files_path`` to be relative to it.
1404+
"""Rewrite absolute path values under the compute-side extra-files dir.
14021405
14031406
Some data managers record the absolute path inside the (transient) job
14041407
directory (e.g. MetaPhlAn's ``$out_file.extra_files_path/$index``) rather
14051408
than the expected relative path; that absolute path no longer resolves once
14061409
the bundle is staged elsewhere, so imports and chained data managers fail.
1407-
Paths already relative, or outside ``extra_files_path``, are left untouched.
1410+
Paths already relative, or outside that directory, are left untouched.
14081411
"""
14091412
for data_table_name, data_table_values in data_tables.items():
14101413
path_headers = get_path_headers(bundle_description, data_table_name)
@@ -1441,9 +1444,6 @@ def _data_manager_dict(out_data: dict[str, OutputDataset], ensure_single_output:
14411444
return data_manager_dict
14421445

14431446

1444-
from collections.abc import Mapping
1445-
1446-
14471447
def _process_bundle(
14481448
out_data: Mapping[str, HasExtraFiles],
14491449
bundle: DataTableBundle,

lib/galaxy/tools/__init__.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
ToolInputsNotReadyException,
4444
)
4545
from galaxy.job_execution import output_collect
46+
from galaxy.job_execution.compute_environment import dataset_path_to_extra_path
4647
from galaxy.job_execution.output_collect import (
4748
BaseJobContext,
4849
MetadataSourceProvider,
@@ -200,6 +201,7 @@
200201
from galaxy.tools.parameters.workflow_utils import workflow_building_modes
201202
from galaxy.tools.parameters.wrapped_json import json_wrap
202203
from galaxy.util import (
204+
asbool,
203205
in_directory,
204206
Params,
205207
parse_xml_string,
@@ -3738,6 +3740,36 @@ def job_failed(self, job_wrapper, message, exception=False):
37383740
self.__remove_interactivetool_by_job(job)
37393741

37403742

3743+
def _data_manager_bundle_source_extra_files_paths(app, job, out_data: dict[str, Any]) -> dict[str, str]:
3744+
"""Return the compute-side extra-files roots for bundle outputs.
3745+
3746+
Data managers running with ``outputs_to_working_directory`` receive an
3747+
extra-files path derived from the job working directory, not the path the
3748+
handler later obtains from the object store. Reconstruct that same path
3749+
here, using the rules in ``OutputsToWorkingDirectoryPathRewriter``.
3750+
"""
3751+
outputs_to_working_directory = asbool(
3752+
job.get_destination_configuration({}, app.config, "outputs_to_working_directory", False)
3753+
)
3754+
if not outputs_to_working_directory:
3755+
return {}
3756+
3757+
job_working_directory = app.object_store.get_filename(job, base_dir="job_work", dir_only=True, obj_dir=True)
3758+
outputs_directory = None if Version(job.galaxy_version or "19.05") < Version("20.01") else "outputs"
3759+
if outputs_directory is not None:
3760+
job_working_directory = os.path.join(job_working_directory, outputs_directory)
3761+
3762+
source_extra_files_paths = {}
3763+
for output_name, dataset in out_data.items():
3764+
if dataset.ext != "data_manager_json":
3765+
continue
3766+
hda = cast(HistoryDatasetAssociation, dataset)
3767+
assert hda.dataset is not None
3768+
output_path = os.path.join(job_working_directory, f"dataset_{hda.dataset.uuid}.dat")
3769+
source_extra_files_paths[output_name] = dataset_path_to_extra_path(output_path)
3770+
return source_extra_files_paths
3771+
3772+
37413773
class DataManagerTool(OutputParameterJSONTool):
37423774
tool_type = "manage_data"
37433775
default_tool_action = DataManagerToolAction
@@ -3768,7 +3800,8 @@ def exec_after_process(self, app, inp_data, out_data, param_dict, job, final_job
37683800
elif data_manager_mode == "dry_run":
37693801
pass
37703802
elif data_manager_mode == "bundle":
3771-
for bundle_path, dataset in data_manager.write_bundle(out_data).items():
3803+
source_extra_files_paths = _data_manager_bundle_source_extra_files_paths(app, job, out_data)
3804+
for bundle_path, dataset in data_manager.write_bundle(out_data, source_extra_files_paths).items():
37723805
hda = cast(HistoryDatasetAssociation, dataset)
37733806
assert hda.dataset is not None
37743807
assert hda.dataset.object_store is not None

lib/galaxy/tools/data_manager/manager.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import errno
22
import logging
33
import os
4+
from collections.abc import Mapping
45
from typing import (
56
Optional,
67
)
@@ -242,12 +243,14 @@ def process_result(self, out_data: dict[str, OutputDataset]) -> None:
242243
def write_bundle(
243244
self,
244245
out_data: dict[str, OutputDataset],
246+
source_extra_files_paths: Mapping[str, str] | None = None,
245247
) -> dict[str, OutputDataset]:
246248
tool_data_tables = self.data_managers.app.tool_data_tables
247249
return tool_data_tables.write_bundle(
248250
out_data,
249251
self.processor_description,
250252
self.repo_info,
253+
source_extra_files_paths,
251254
)
252255

253256
@property

test/unit/tool_util/data/test_tool_data_bundles.py

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,150 @@ def test_import_bundle_with_absolute_recorded_path(tdt_manager, tmp_path, table_
309309
assert os.path.exists(os.path.join(loc_target, f"{index}.pkl"))
310310

311311

312+
def prepare_split_prefix_output_and_description(
313+
tmp_path,
314+
table_name="testalpha",
315+
path_column="path",
316+
store_leaf=None,
317+
compute_leaf=None,
318+
recorded_relative_path=None,
319+
):
320+
"""Model the object-store vs job-working-dir prefix split.
321+
322+
In production ``write_bundle`` runs on the job handler, where
323+
``dataset.extra_files_path`` resolves to the object-store extra-files dir,
324+
while the data manager baked its absolute ``path`` on the compute node under
325+
the transient job working dir. The directories differ in every prefix
326+
component, so a plain ``os.path.relpath`` escapes with ``..``. They may even
327+
differ in the ``dataset_<key>_files`` leaf itself: the compute side is
328+
uuid-keyed under outputs_to_working_directory while the store may be
329+
id-keyed — pass distinct ``store_leaf``/``compute_leaf`` to model that. The
330+
recorded absolute path deliberately does NOT exist under
331+
``extra_files_path`` here, so the fix must be structural (no existence check).
332+
"""
333+
index = "mpa_toy"
334+
compute_leaf = compute_leaf or "dataset_00000000-0000-0000-0000-000000000000_files"
335+
store_leaf = store_leaf or compute_leaf
336+
337+
# Object-store extra-files dir: where the bundle index actually gets written.
338+
extra_files_path = tmp_path / "objectstore" / "000" / store_leaf
339+
extra_files_path.mkdir(parents=True)
340+
341+
# Absolute path as recorded by the DM under the (now-irrelevant) job working dir.
342+
job_dir = tmp_path / "jobs" / "001" / "outputs" / compute_leaf
343+
recorded_path = str(job_dir / (index if recorded_relative_path is None else recorded_relative_path))
344+
345+
output = {"data_tables": {table_name: [{"value": index, "name": "toy", path_column: recorded_path}]}}
346+
output_dataset_path = tmp_path / "output.dat"
347+
output_dataset_path.write_text(json.dumps(output))
348+
output_dataset = OutputDataset(output_dataset_path, extra_files_path)
349+
out_data = {"out1": output_dataset}
350+
data_table = {
351+
"name": table_name,
352+
"output": {
353+
"columns": [
354+
{"name": "value"},
355+
{"name": "name"},
356+
{
357+
"name": path_column,
358+
"output_ref": "out1",
359+
"moves": [
360+
{
361+
"type": "directory",
362+
"relativize_symlinks": False,
363+
"source_value": "${" + path_column + "}",
364+
"target_value": "metaphlan/data/${value}",
365+
"target_base": "${GALAXY_DATA_MANAGER_DATA_PATH}",
366+
}
367+
],
368+
"value_translations": [
369+
{"value": "${GALAXY_DATA_MANAGER_DATA_PATH}/metaphlan/data/${value}", "type": "template"},
370+
{"value": "abspath", "type": "function"},
371+
],
372+
},
373+
]
374+
},
375+
}
376+
process_description = DataTableBundleProcessorDescription(
377+
**{"undeclared_tables": False, "data_tables": [data_table]}
378+
)
379+
return index, extra_files_path, job_dir, out_data, process_description
380+
381+
382+
def test_write_bundle_relativizes_split_prefix_path_from_compute_root(tdt_manager, tmp_path):
383+
"""A data manager's compute-side path is made relative to its known job-dir root."""
384+
index, extra_files_path, compute_extra_files_path, out_data, process_description = (
385+
prepare_split_prefix_output_and_description(tmp_path)
386+
)
387+
tdt_manager.write_bundle(
388+
out_data,
389+
process_description,
390+
None,
391+
source_extra_files_paths={"out1": str(compute_extra_files_path)},
392+
)
393+
394+
bundle_index = json.loads((extra_files_path / BUNDLE_INDEX_FILE_NAME).read_text())
395+
stored_path = bundle_index["data_tables"]["testalpha"][0]["path"]
396+
assert stored_path == index
397+
assert not os.path.isabs(stored_path)
398+
399+
400+
def test_write_bundle_relativizes_uuid_keyed_job_path_on_id_keyed_store(tdt_manager, tmp_path):
401+
"""A uuid-keyed compute root works even when the object store is id-keyed."""
402+
index, extra_files_path, compute_extra_files_path, out_data, process_description = (
403+
prepare_split_prefix_output_and_description(
404+
tmp_path,
405+
store_leaf="dataset_42_files",
406+
compute_leaf="dataset_00000000-0000-0000-0000-000000000000_files",
407+
)
408+
)
409+
tdt_manager.write_bundle(
410+
out_data,
411+
process_description,
412+
None,
413+
source_extra_files_paths={"out1": str(compute_extra_files_path)},
414+
)
415+
416+
bundle_index = json.loads((extra_files_path / BUNDLE_INDEX_FILE_NAME).read_text())
417+
stored_path = bundle_index["data_tables"]["testalpha"][0]["path"]
418+
assert stored_path == index
419+
assert not os.path.isabs(stored_path)
420+
421+
422+
def test_write_bundle_relativizes_compute_extra_files_root(tdt_manager, tmp_path):
423+
_, extra_files_path, compute_extra_files_path, out_data, process_description = (
424+
prepare_split_prefix_output_and_description(tmp_path, recorded_relative_path="")
425+
)
426+
tdt_manager.write_bundle(
427+
out_data,
428+
process_description,
429+
None,
430+
source_extra_files_paths={"out1": str(compute_extra_files_path)},
431+
)
432+
433+
bundle_index = json.loads((extra_files_path / BUNDLE_INDEX_FILE_NAME).read_text())
434+
assert bundle_index["data_tables"]["testalpha"][0]["path"] == os.curdir
435+
436+
437+
def test_write_bundle_preserves_nested_dataset_named_directory(tdt_manager, tmp_path):
438+
store_leaf = "dataset_42_files"
439+
relative_path = os.path.join("mpa_toy", store_leaf, "index")
440+
_, extra_files_path, compute_extra_files_path, out_data, process_description = (
441+
prepare_split_prefix_output_and_description(
442+
tmp_path, store_leaf=store_leaf, recorded_relative_path=relative_path
443+
)
444+
)
445+
tdt_manager.write_bundle(
446+
out_data,
447+
process_description,
448+
None,
449+
source_extra_files_paths={"out1": str(compute_extra_files_path)},
450+
)
451+
452+
bundle_index = json.loads((extra_files_path / BUNDLE_INDEX_FILE_NAME).read_text())
453+
assert bundle_index["data_tables"]["testalpha"][0]["path"] == relative_path
454+
455+
312456
def test_path_headers_from_move_and_abspath():
313457
"""A column is a path column when it has a move or an abspath translation, whatever its name."""
314458
description = DataTableBundleProcessorDescription(

0 commit comments

Comments
 (0)