Skip to content

Commit

Permalink
Merge branch 'main' into dependabot/pip/furo-2024.8.6
Browse files Browse the repository at this point in the history
  • Loading branch information
khsrali authored Sep 17, 2024
2 parents be79d50 + a3d918b commit 5b8d59e
Show file tree
Hide file tree
Showing 11 changed files with 331 additions and 89 deletions.
3 changes: 2 additions & 1 deletion .firecrest-demo-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
"temp_directory": "",
"small_file_size_mb": 5.0,
"workdir": "",
"api_version": "1.16.0"
"api_version": "1.16.0",
"builder_metadata_options_custom_scheduler_commands": []
}
8 changes: 4 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ repos:
- id: trailing-whitespace

- repo: https://github.com/asottile/pyupgrade
rev: v3.16.0
rev: v3.17.0
hooks:
- id: pyupgrade
args: [--py39-plus]
Expand All @@ -27,17 +27,17 @@ repos:
- id: isort

- repo: https://github.com/psf/black
rev: 24.4.2
rev: 24.8.0
hooks:
- id: black

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.0
rev: v0.6.3
hooks:
- id: ruff

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.10.1
rev: v1.11.2
hooks:
- id: mypy
files: ^(aiida_firecrest/.*py)$
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,6 @@ If these tests pass and the bug persists, consider providing a `config` file to

If the bug persists and test still passes, then most certainly it's a problem of `aiida-firecrest`.
If not, probably the issue is from FirecREST, you might open an issue to [pyfirecrest](https://github.com/eth-cscs/pyfirecrest) or [`FirecREST`](https://github.com/eth-cscs/firecrest).

### Acknowledgment:
This project is supported by SwissTwins fundings.
21 changes: 9 additions & 12 deletions aiida_firecrest/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@


class FirecrestScheduler(Scheduler):
"""Scheduler interface for FirecREST."""
"""Scheduler interface for FirecREST.
It must be used together with the 'firecrest' transport plugin.
"""

transport: FirecrestTransport
_job_resource_class = SlurmJobResource
Expand All @@ -32,14 +34,6 @@ class FirecrestScheduler(Scheduler):
_logger = Scheduler._logger.getChild("firecrest")
_DEFAULT_PAGE_SIZE = 25

@classmethod
def get_description(cls) -> str:
"""Used by verdi to describe the plugin."""
return (
"A plugin to connect to a FirecREST server.\n"
"It must be used together with the 'firecrest' transport plugin.\n"
)

def _get_submit_script_header(self, job_tmpl: JobTemplate) -> str:
"""
Return the submit script header, using the parameters from the
Expand Down Expand Up @@ -221,14 +215,17 @@ def get_jobs(
try:
for page_iter in itertools.count():
results += transport._client.poll_active(
transport._machine, jobs, page_number=page_iter
transport._machine,
jobs,
page_number=page_iter,
page_size=self._DEFAULT_PAGE_SIZE,
)
if len(results) < self._DEFAULT_PAGE_SIZE * (page_iter + 1):
break
except FirecrestException as 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):
if "Invalid job id" not in str(exc):
# firecrest returns error if the job is completed, while aiida expect a silent return
raise SchedulerError(str(exc)) from exc
job_list = []
for raw_result in results:
Expand Down
77 changes: 49 additions & 28 deletions aiida_firecrest/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,28 @@ def _validate_temp_directory(ctx: Context, param: InteractiveOption, value: str)
def _dynamic_info_firecrest_version(
ctx: Context, param: InteractiveOption, value: str
) -> str:
"""Find the version of the FirecREST server."""
# note: right now, unfortunately, the version is not exposed in the API.
# See issue https://github.com/eth-cscs/firecrest/issues/204
# so here we just develope a workaround to get the version from the server
# basically we check if extract/compress endpoint is available
"""
Find the version of the FirecREST server.
This is a callback function for click, interface.
It's called during the parsing of the command line arguments, during `verdi computer configure` command.
If the user enters "None" as value, this function will connect to the server and get the version of the FirecREST server.
Otherwise, it will check if the version is supported.
"""

import click
from packaging.version import InvalidVersion

if value != "None":
try:
parse(value)
except InvalidVersion as err:
# raise in case the version is not valid, e.g. latest, stable, etc.
raise click.BadParameter(f"Invalid input {value}") from err

if value != "0":
if parse(value) < parse("1.15.0"):
raise click.BadParameter(f"FirecREST api version {value} is not supported")
# If version is provided by the user, and it's supported, we will just return it.
# No print confirmation is needed, to keep things less verbose.
return value

firecrest_url = ctx.params["url"]
Expand All @@ -158,16 +169,32 @@ def _dynamic_info_firecrest_version(
small_file_size_mb=0.0,
api_version="100.0.0", # version is irrelevant here
)

parameters = transport._client.parameters()
try:
transport.listdir(transport._cwd.joinpath(temp_directory), recursive=True)
_version = "1.16.0"
except Exception:
# all sort of exceptions can be raised here, but we don't care. Since this is just a workaround
_version = "1.15.0"
info = next(
(
item
for item in parameters["general"] # type: ignore[typeddict-item]
if item["name"] == "FIRECREST_VERSION"
),
None,
)
if info is not None:
_version = str(info["value"])
else:
raise KeyError
except KeyError as err:
click.echo("Could not get the version of the FirecREST server")
raise click.Abort() from err

if parse(_version) < parse("1.15.0"):
click.echo(f"FirecREST api version {_version} is not supported")
raise click.Abort()

# for the sake of uniformity, we will print the version in style only if dynamically retrieved.
click.echo(
click.style("Fireport: ", bold=True, fg="magenta")
+ f"Deployed version of FirecREST api: v{_version}"
click.style("Fireport: ", bold=True, fg="magenta") + f"FirecRESTapi: {_version}"
)
return _version

Expand Down Expand Up @@ -233,7 +260,8 @@ def _dynamic_info_direct_size(


class FirecrestTransport(Transport):
"""Transport interface for FirecREST."""
"""Transport interface for FirecREST.
It must be used together with the 'firecrest' scheduler plugin."""

# override these options, because they don't really make sense for a REST-API,
# so we don't want the user having to provide them
Expand Down Expand Up @@ -307,7 +335,7 @@ class FirecrestTransport(Transport):
"api_version",
{
"type": str,
"default": "0",
"default": "None",
"non_interactive_default": True,
"prompt": "FirecREST api version [Enter 0 to get this info from server]",
"help": "The version of the FirecREST api deployed on the server",
Expand Down Expand Up @@ -401,6 +429,8 @@ def __init__(
# aiida-core/src/aiida/orm/utils/remote:clean_remote()
self._is_open = True

self.checksum_check = False

def __str__(self) -> str:
"""Return the name of the plugin."""
return self.__class__.__name__
Expand All @@ -419,17 +449,6 @@ def payoff_override(self, value: bool) -> None:
raise ValueError("payoff_override must be a boolean value")
self._payoff_override = value

@classmethod
def get_description(cls) -> str:
"""Used by verdi to describe the plugin."""
return (
"A plugin to connect to a FirecREST server.\n"
"It must be used together with the 'firecrest' scheduler plugin.\n"
"Authentication parameters:\n"
) + "\n".join(
[f" {k}: {v.get('help', '')}" for k, v in cls.auth_options.items()]
)

def open(self) -> None:
"""Open the transport.
This is a no-op for the REST-API, as there is no connection to open.
Expand Down Expand Up @@ -723,7 +742,8 @@ def getfile(
down_obj = self._client.external_download(self._machine, str(remote))
down_obj.finish_download(local)

self._validate_checksum(local, remote)
if self.checksum_check:
self._validate_checksum(local, remote)

def _validate_checksum(
self, localpath: str | Path, remotepath: str | FcPath
Expand Down Expand Up @@ -965,7 +985,8 @@ def putfile(
)
up_obj.finish_upload()

self._validate_checksum(localpath, str(remote))
if self.checksum_check:
self._validate_checksum(localpath, str(remote))

def payoff(self, path: str | FcPath | Path) -> bool:
"""
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ exclude = [

[tool.ruff]
line-length = 120
extend-select = ["A", "B", "C4", "ICN", "ISC", "N", "RUF", "SIM"]
lint.extend-select = ["A", "B", "C4", "ICN", "ISC", "N", "RUF", "SIM"]
# extend-ignore = []

[tool.isort]
Expand Down
Loading

0 comments on commit 5b8d59e

Please sign in to comment.