Skip to content

Commit aff3d71

Browse files
committed
🐛 COPY stash handles missing sources unlike the compressed modes
`RemoteStashFolderData.source_list` documents "the source files that were stashed", but with `fail_on_missing=False` it kept the entries skipped for being missing: record only what was copied. With `fail_on_missing=True` a non-matching glob silently copied nothing: reject glob patterns, as the compressed modes do.
1 parent d0dd569 commit aff3d71

2 files changed

Lines changed: 50 additions & 10 deletions

File tree

src/aiida/engine/daemon/execmanager.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,16 +501,24 @@ async def stash_calculation(calculation: CalcJobNode, transport: Transport) -> N
501501
else:
502502
target_basepath = target_base / uuid[:2] / uuid[2:4] / uuid[4:]
503503

504-
async def _do_copy():
504+
async def _do_copy() -> list[str]:
505+
stashed_source_list: list[str] = []
505506
for source_filename in source_list:
506507
if has_magic(source_filename):
508+
if fail_on_missing:
509+
msg = (
510+
'Stashing with glob patterns is not supported when fail_on_missing is True. '
511+
'Stashing failed.'
512+
)
513+
raise exceptions.StashingError(msg)
507514
copy_instructions = []
508515
for globbed_filename in await transport.glob_async(source_basepath / source_filename):
509516
target_filepath = target_basepath / Path(globbed_filename).relative_to(source_basepath)
510517
copy_instructions.append((globbed_filename, target_filepath))
511518
else:
512519
copy_instructions = [(source_basepath / source_filename, target_basepath / source_filename)]
513520

521+
stashed = False
514522
for source_filepath, target_filepath in copy_instructions:
515523
# If source is in a (nested) directory, create those directories first
516524
target_dirname = target_filepath.parent
@@ -533,9 +541,13 @@ async def _do_copy():
533541
f'Failed to copy {source_filepath} to {target_filepath}: {exc}'
534542
) from exc
535543
EXEC_LOGGER.debug(f'Stashed from {source_filepath} to {target_filepath}')
544+
stashed = True
545+
if stashed:
546+
stashed_source_list.append(source_filename)
547+
return stashed_source_list
536548

537549
try:
538-
await _do_copy()
550+
stashed_source_list = await _do_copy()
539551
except exceptions.StashingError as exception:
540552
await transport.rmtree_async(target_basepath)
541553
raise exception
@@ -546,7 +558,7 @@ async def _do_copy():
546558
computer=calculation.computer,
547559
target_basepath=str(target_basepath),
548560
stash_mode=StashMode(stash_mode),
549-
source_list=source_list,
561+
source_list=stashed_source_list,
550562
fail_on_missing=fail_on_missing,
551563
).store()
552564

tests/engine/daemon/test_execmanager.py

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -811,9 +811,10 @@ async def mock_compress_partial(*args, **kwargs):
811811
# Overwrites: a job can only replace its own leftover, see ``test_stashing_same_source_twice``
812812

813813

814+
@pytest.mark.parametrize('stash_mode', [StashMode.COPY.value, StashMode.COMPRESS_TARGZ.value])
814815
@pytest.mark.asyncio
815-
async def test_stashing_compress_skips_missing(generate_calcjob_node, tmp_path, monkeypatch):
816-
"""With ``fail_on_missing=False`` a compressed stash skips missing sources instead of failing silently."""
816+
async def test_stashing_skips_missing(generate_calcjob_node, stash_mode, tmp_path, monkeypatch):
817+
"""With ``fail_on_missing=False`` missing sources are skipped and the stash node records only what was stashed."""
817818
node = generate_calcjob_node()
818819
workdir = tmp_path / 'workdir'
819820
workdir.mkdir()
@@ -825,7 +826,7 @@ async def test_stashing_compress_skips_missing(generate_calcjob_node, tmp_path,
825826
{
826827
'source_list': ['present.out', 'missing.out', 'nomatch*'],
827828
'target_base': str(target_base),
828-
'stash_mode': StashMode.COMPRESS_TARGZ.value,
829+
'stash_mode': stash_mode,
829830
},
830831
)
831832

@@ -838,15 +839,42 @@ def get_workdir(self, *args, **kwargs):
838839

839840
with LocalTransport() as transport:
840841
await execmanager.stash_calculation(node, transport)
841-
transport.extract(target_base / f'{node.uuid}.tar.gz', tmp_path / 'extracted')
842-
843-
assert [path.name for path in (tmp_path / 'extracted').iterdir()] == ['present.out']
842+
if stash_mode == StashMode.COPY.value:
843+
stashed = target_base / node.uuid[:2] / node.uuid[2:4] / node.uuid[4:]
844+
else:
845+
stashed = tmp_path / 'extracted'
846+
transport.extract(target_base / f'{node.uuid}.{stash_mode}', stashed)
844847

845-
# The stash node must record only what actually went into the archive
848+
assert [path.name for path in stashed.iterdir()] == ['present.out']
846849
remote_stash = node.base.links.get_outgoing(link_label_filter='remote_stash').one().node
847850
assert list(remote_stash.source_list) == ['present.out']
848851

849852

853+
@pytest.mark.parametrize('stash_mode', [StashMode.COPY.value, StashMode.COMPRESS_TARGZ.value])
854+
@pytest.mark.asyncio
855+
async def test_stashing_fail_on_missing_rejects_glob(generate_calcjob_node, stash_mode, tmp_path, monkeypatch):
856+
"""With ``fail_on_missing=True`` glob patterns are rejected, since a non-matching one cannot be told apart."""
857+
node = generate_calcjob_node(workdir=tmp_path)
858+
node.set_option(
859+
'stash',
860+
{
861+
'source_list': ['*.out'],
862+
'target_base': str(tmp_path / 'stash'),
863+
'stash_mode': stash_mode,
864+
'fail_on_missing': True,
865+
},
866+
)
867+
868+
class MockAuthInfo:
869+
def get_workdir(self, *args, **kwargs):
870+
return str(tmp_path)
871+
872+
monkeypatch.setattr(node, 'get_authinfo', MockAuthInfo)
873+
874+
with LocalTransport() as transport, pytest.raises(StashingError, match='glob patterns'):
875+
await execmanager.stash_calculation(node, transport)
876+
877+
850878
@pytest.mark.parametrize('stash_mode', [StashMode.COPY.value, StashMode.COMPRESS_TARGZ.value])
851879
@pytest.mark.asyncio
852880
async def test_stashing_same_source_twice(generate_calcjob_node, aiida_localhost, stash_mode, tmp_path, monkeypatch):

0 commit comments

Comments
 (0)