Skip to content

Commit

Permalink
bug fix: when get_job() is retiereving a completed job
Browse files Browse the repository at this point in the history
bug fix: iglob() is sending relative path to listdir()
compatibility with new version of firecrest: extract() and compress()  will take care of submission if needed
  • Loading branch information
khsrali committed Jun 27, 2024
1 parent 3c3773f commit 96fe286
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 33 deletions.
6 changes: 5 additions & 1 deletion aiida_firecrest/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ def get_jobs(
) -> list[JobInfo] | dict[str, JobInfo]:
results = []
transport = self.transport

with convert_header_exceptions({"machine": transport._machine}):
# TODO handle pagination (pageSize, pageNumber) if many jobs
# This will do pagination
Expand All @@ -224,7 +225,10 @@ def get_jobs(
if len(results) < self._DEFAULT_PAGE_SIZE * (page_iter + 1):
break
except FirecrestException as exc:
raise SchedulerError(str(exc)) from exc
# firecrest returns error if the job is completed
# TODO: check what type of error is returned and handle it properly
if "Invalid job id specified" not in str(exc):
raise SchedulerError(str(exc)) from exc
job_list = []
for raw_result in results:
# TODO: probably the if below is not needed, because recently
Expand Down
45 changes: 13 additions & 32 deletions aiida_firecrest/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from pathlib import Path
import posixpath
import tarfile
import time
from typing import Any, Callable, ClassVar, TypedDict
import uuid

Expand Down Expand Up @@ -441,16 +440,20 @@ def listdir(
) -> list[str]:
"""List the contents of a directory.
:param path: this could be relative or absolute path.
Note igolb() will usually call this with relative path.
:param pattern: Unix shell-style wildcards to match the pattern:
- `*` matches everything
- `?` matches any single character
- `[seq]` matches any character in seq
- `[!seq]` matches any character not in seq
:param recursive: If True, list directories recursively
"""
path_abs = self._cwd.joinpath(path)
names = [
p.relpath(path).as_posix()
for p in self._cwd.joinpath(path).iterdir(recursive=recursive)
p.relpath(path_abs).as_posix()
for p in path_abs.iterdir(recursive=recursive)
]
if pattern is not None:
names = fnmatch.filter(names, pattern)
Expand Down Expand Up @@ -609,7 +612,6 @@ def getfile(
note: we don't support downloading symlinks, so dereference should always be True
"""

if not dereference:
raise NotImplementedError(
"Getting symlinks with `dereference=False` is not supported"
Expand Down Expand Up @@ -705,25 +707,12 @@ def _gettreetar(
note: FirecREST doesn't support `--dereference` for tar call,
so dereference should always be False, for now.
"""
# TODO manual testing the submit behaviour

# if dereference:
# raise NotImplementedError("Dereferencing compression not implemented in pyFirecREST.")

_ = uuid.uuid4() # Attempt direct compress
_ = uuid.uuid4()
remote_path_temp = self._temp_directory.joinpath(f"temp_{_}.tar")
try:
self._client.compress(self._machine, str(remotepath), remote_path_temp)
except Exception as e:
# TODO: pyfirecrest is providing a solution to this, but it's not yet merged.
# once done submit_compress_job should be done automaticaly by compress
# see: https://github.com/eth-cscs/pyfirecrest/pull/109
raise NotImplementedError("Not implemeted for now") from e
comp_obj = self._client.submit_compress_job(
self._machine, str(remotepath), remote_path_temp
)
while comp_obj.in_progress:
time.sleep(self._file_transfer_poll_interval)

# Compress
self._client.compress(self._machine, str(remotepath), remote_path_temp)

# Download
localpath_temp = Path(localpath).joinpath(f"temp_{_}.tar")
Expand Down Expand Up @@ -756,6 +745,7 @@ def gettree(
:param dereference: If True, follow symlinks.
note: dereference should be always True, otherwise the symlinks will not be functional.
"""

local = Path(localpath)
if not local.is_absolute():
raise ValueError("Destination must be an absolute path")
Expand Down Expand Up @@ -947,19 +937,10 @@ def _puttreetar(
self.putfile(tarpath, remote_path_temp)
finally:
tarpath.unlink()
# Attempt direct extract

# Attempt extract
try:
self._client.extract(self._machine, remote_path_temp, str(remotepath))
except Exception as e:
# TODO: pyfirecrest is providing a solution to this, but it's not yet merged
# once done submit_compress_job should be done automaticaly by compress
# see: https://github.com/eth-cscs/pyfirecrest/pull/109
raise NotImplementedError("Not implemeted for now") from e
comp_obj = self._client.submit_extract_job(
self._machine, remotepath.joinpath(f"_{_}.tar"), str(remotepath)
)
while comp_obj.in_progress:
time.sleep(self._file_transfer_poll_interval)
finally:
self.remove(remote_path_temp)

Expand Down

0 comments on commit 96fe286

Please sign in to comment.