Skip to content

Commit

Permalink
Add support for running direct jobs on bash environment on Windows
Browse files Browse the repository at this point in the history
Fixes to run aiida within an bash environment on windows: 
* Path support for windows: 
  - `pathlib.PosixPurePath` -> `pathlib.PurePath`
  - usage of `url2pathname` and `pathlib.Path(..).as_uri()`
  - `os.rename` to `shutil.move`, as rename and moving is the same on Posix but
    is different on Windows
* Windows does not use utf-8 for encoding pipes by default. Too detect the
  used encoding of stdout and stderr we use `chardet`.
  • Loading branch information
lainme authored Feb 17, 2025
1 parent c4dfada commit ce9dcf4
Show file tree
Hide file tree
Showing 27 changed files with 102 additions and 60 deletions.
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ dependencies:
- typing-extensions~=4.0
- upf_to_json~=0.9.2
- wrapt~=1.11
- chardet~=5.2.0 # [win]
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ dependencies = [
'tqdm~=4.45',
'typing-extensions~=4.0;python_version<"3.10"',
'upf_to_json~=0.9.2',
'wrapt~=1.11'
'wrapt~=1.11',
'chardet~=5.2.0;platform_system=="Windows"'
]
description = 'AiiDA is a workflow manager for computational science with a strong focus on provenance, performance and extensibility.'
dynamic = ['version'] # read from aiida/__init__.py
Expand Down
4 changes: 3 additions & 1 deletion src/aiida/cmdline/commands/cmd_computer.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,14 @@ def _computer_create_temp_file(transport, scheduler, authinfo, computer):

transport.makedirs(workdir, ignore_existing=True)

with tempfile.NamedTemporaryFile(mode='w+') as tempf:
with tempfile.NamedTemporaryFile(mode='w+', delete=False) as tempf:
fname = os.path.split(tempf.name)[1]
remote_file_path = os.path.join(workdir, fname)
tempf.write(file_content)
tempf.flush()
tempf.close()
transport.putfile(tempf.name, remote_file_path)
os.remove(tempf.name)

if not transport.path_exists(remote_file_path):
return False, f'failed to create the file `{remote_file_path}` on the remote'
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/cmdline/commands/cmd_presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def detect_postgres_config(
'database_name': database_name,
'database_username': database_username,
'database_password': database_password,
'repository_uri': f'file://{aiida_config_folder / "repository" / profile_name}',
'repository_uri': pathlib.Path(f'{aiida_config_folder / "repository" / profile_name}').as_uri(),
}


Expand Down
4 changes: 3 additions & 1 deletion src/aiida/cmdline/commands/cmd_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
###########################################################################
"""The `verdi setup` and `verdi quicksetup` commands."""

import pathlib

import click

from aiida.cmdline.commands.cmd_verdi import verdi
Expand Down Expand Up @@ -88,7 +90,7 @@ def setup(
'database_name': db_name,
'database_username': db_username,
'database_password': db_password,
'repository_uri': f'file://{repository}',
'repository_uri': pathlib.Path(f'{repository}').as_uri(),
},
)
profile.set_process_controller(
Expand Down
5 changes: 4 additions & 1 deletion src/aiida/engine/daemon/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ class DaemonClient:
"""Client to interact with the daemon."""

_DAEMON_NAME = 'aiida-{name}'
_ENDPOINT_PROTOCOL = ControllerProtocol.IPC
if sys.platform == 'win32':
_ENDPOINT_PROTOCOL = ControllerProtocol.TCP
else:
_ENDPOINT_PROTOCOL = ControllerProtocol.IPC

def __init__(self, profile: Profile):
"""Construct an instance for a given profile.
Expand Down
4 changes: 3 additions & 1 deletion src/aiida/manage/configuration/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io
import json
import os
import shutil
import uuid
from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple
Expand Down Expand Up @@ -780,7 +781,8 @@ def _atomic_write(self, filepath=None):
os.umask(umask)

handle.flush()
os.rename(handle.name, self.filepath)
handle.close()
shutil.move(handle.name, self.filepath)

def filepaths(self, profile: Profile):
"""Return the filepaths used by a profile.
Expand Down
5 changes: 3 additions & 2 deletions src/aiida/manage/configuration/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ def repository_path(self) -> pathlib.Path:
:return: absolute filepath of the profile's file repository
"""
from urllib.parse import urlparse
from urllib.request import url2pathname

from aiida.common.warnings import warn_deprecation

Expand All @@ -224,10 +225,10 @@ def repository_path(self) -> pathlib.Path:
if parts.scheme != 'file':
raise exceptions.ConfigurationError('invalid repository protocol, only the local `file://` is supported')

if not os.path.isabs(parts.path):
if not os.path.isabs(url2pathname(parts.path)):
raise exceptions.ConfigurationError('invalid repository URI: the path has to be absolute')

return pathlib.Path(os.path.expanduser(parts.path))
return pathlib.Path(os.path.expanduser(url2pathname(parts.path)))

@property
def filepaths(self):
Expand Down
3 changes: 2 additions & 1 deletion src/aiida/manage/profile_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import contextlib
import os
import shutil
import typing
from pathlib import Path

Expand Down Expand Up @@ -76,7 +77,7 @@ def request_access(self) -> None:
# it will read the incomplete command, won't be able to correctly compare it with its running
# process, and will conclude the record is old and clean it up.
filepath_tmp.write_text(str(self.process.cmdline()))
os.rename(filepath_tmp, filepath_pid)
shutil.move(filepath_tmp, filepath_pid)

# Check again in case a lock was created in the time between the first check and creating the
# access record file.
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/manage/tests/pytest_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def factory(custom_configuration: dict[str, t.Any] | None = None) -> dict[str, t
'storage': {
'backend': 'core.psql_dos',
'config': {
'repository_uri': f'file://{tmp_path_factory.mktemp("repository")}',
'repository_uri': pathlib.Path(f'{tmp_path_factory.mktemp("repository")}').as_uri(),
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/code/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def can_run_on_computer(self, computer: Computer) -> bool:
"""

@abc.abstractmethod
def get_executable(self) -> pathlib.PurePosixPath:
def get_executable(self) -> pathlib.PurePath:
"""Return the executable that the submission script should execute to run the code.
:return: The executable to be called in the submission script.
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/code/containerized.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def __init__(self, engine_command: str, image_name: str, **kwargs):
self.image_name = image_name

@property
def filepath_executable(self) -> pathlib.PurePosixPath:
def filepath_executable(self) -> pathlib.PurePath:
"""Return the filepath of the executable that this code represents.
.. note:: This is overridden from the base class since the path does not have to be absolute.
Expand Down
6 changes: 3 additions & 3 deletions src/aiida/orm/nodes/data/code/installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def can_run_on_computer(self, computer: Computer) -> bool:
type_check(computer, Computer)
return computer.pk == self.computer.pk

def get_executable(self) -> pathlib.PurePosixPath:
def get_executable(self) -> pathlib.PurePath:
"""Return the executable that the submission script should execute to run the code.
:return: The executable to be called in the submission script.
Expand Down Expand Up @@ -207,12 +207,12 @@ def full_label(self) -> str:
return f'{self.label}@{self.computer.label}'

@property
def filepath_executable(self) -> pathlib.PurePosixPath:
def filepath_executable(self) -> pathlib.PurePath:
"""Return the absolute filepath of the executable that this code represents.
:return: The absolute filepath of the executable.
"""
return pathlib.PurePosixPath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE))
return pathlib.PurePath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE))

@filepath_executable.setter
def filepath_executable(self, value: str) -> None:
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/orm/nodes/data/code/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def can_run_on_computer(self, computer: Computer) -> bool:
type_check(computer, orm.Computer)
return computer.pk == self.get_remote_computer().pk

def get_executable(self) -> pathlib.PurePosixPath:
def get_executable(self) -> pathlib.PurePath:
"""Return the executable that the submission script should execute to run the code.
:return: The executable to be called in the submission script.
Expand All @@ -133,7 +133,7 @@ def get_executable(self) -> pathlib.PurePosixPath:
else:
exec_path = self.get_remote_exec_path()

return pathlib.PurePosixPath(exec_path)
return pathlib.PurePath(exec_path)

def hide(self):
"""Hide the code (prevents from showing it in the verdi code list)"""
Expand Down
8 changes: 4 additions & 4 deletions src/aiida/orm/nodes/data/code/portable.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def can_run_on_computer(self, computer: Computer) -> bool:
"""
return True

def get_executable(self) -> pathlib.PurePosixPath:
def get_executable(self) -> pathlib.PurePath:
"""Return the executable that the submission script should execute to run the code.
:return: The executable to be called in the submission script.
Expand Down Expand Up @@ -161,12 +161,12 @@ def full_label(self) -> str:
return self.label

@property
def filepath_executable(self) -> pathlib.PurePosixPath:
def filepath_executable(self) -> pathlib.PurePath:
"""Return the relative filepath of the executable that this code represents.
:return: The relative filepath of the executable.
"""
return pathlib.PurePosixPath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE))
return pathlib.PurePath(self.base.attributes.get(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE))

@filepath_executable.setter
def filepath_executable(self, value: str) -> None:
Expand All @@ -176,7 +176,7 @@ def filepath_executable(self, value: str) -> None:
"""
type_check(value, str)

if pathlib.PurePosixPath(value).is_absolute():
if pathlib.PurePath(value).is_absolute():
raise ValueError('The `filepath_executable` should not be absolute.')

self.base.attributes.set(self._KEY_ATTRIBUTE_FILEPATH_EXECUTABLE, value)
Expand Down
8 changes: 4 additions & 4 deletions src/aiida/orm/nodes/data/folder.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

__all__ = ('FolderData',)

FilePath = t.Union[str, pathlib.PurePosixPath]
FilePath = t.Union[str, pathlib.PurePath]


class FolderData(Data):
Expand Down Expand Up @@ -177,7 +177,7 @@ def put_object_from_tree(self, filepath: str, path: str | None = None) -> None:
"""
return self.base.repository.put_object_from_tree(filepath, path)

def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePosixPath, list[str], list[str]]]:
def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePath, list[str], list[str]]]:
"""Walk over the directories and files contained within this repository.
.. note:: the order of the dirname and filename lists that are returned is not necessarily sorted. This is in
Expand All @@ -186,11 +186,11 @@ def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePos
:param path: the relative path of the directory within the repository whose contents to walk.
:return: tuples of root, dirnames and filenames just like ``os.walk``, with the exception that the root path is
always relative with respect to the repository root, instead of an absolute path and it is an instance of
``pathlib.PurePosixPath`` instead of a normal string
``pathlib.PurePath`` instead of a normal string
"""
yield from self.base.repository.walk(path)

def glob(self) -> t.Iterable[pathlib.PurePosixPath]:
def glob(self) -> t.Iterable[pathlib.PurePath]:
"""Yield a recursive list of all paths (files and directories)."""
yield from self.base.repository.glob()

Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/singlefile.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

__all__ = ('SinglefileData',)

FilePath = t.Union[str, pathlib.PurePosixPath]
FilePath = t.Union[str, pathlib.PurePath]


class SinglefileData(Data):
Expand Down
8 changes: 4 additions & 4 deletions src/aiida/orm/nodes/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

__all__ = ('NodeRepository',)

FilePath = t.Union[str, pathlib.PurePosixPath]
FilePath = t.Union[str, pathlib.PurePath]


class NodeRepository:
Expand Down Expand Up @@ -317,7 +317,7 @@ def put_object_from_tree(self, filepath: str, path: str | None = None):
self._repository.put_object_from_tree(filepath, path)
self._update_repository_metadata()

def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePosixPath, list[str], list[str]]]:
def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePath, list[str], list[str]]]:
"""Walk over the directories and files contained within this repository.
.. note:: the order of the dirname and filename lists that are returned is not necessarily sorted. This is in
Expand All @@ -326,11 +326,11 @@ def walk(self, path: FilePath | None = None) -> t.Iterable[tuple[pathlib.PurePos
:param path: the relative path of the directory within the repository whose contents to walk.
:return: tuples of root, dirnames and filenames just like ``os.walk``, with the exception that the root path is
always relative with respect to the repository root, instead of an absolute path and it is an instance of
``pathlib.PurePosixPath`` instead of a normal string
``pathlib.PurePath`` instead of a normal string
"""
yield from self._repository.walk(path)

def glob(self) -> t.Iterable[pathlib.PurePosixPath]:
def glob(self) -> t.Iterable[pathlib.PurePath]:
"""Yield a recursive list of all paths (files and directories)."""
for dirpath, dirnames, filenames in self.walk():
for dirname in dirnames:
Expand Down
34 changes: 17 additions & 17 deletions src/aiida/repository/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

__all__ = ('Repository',)

FilePath = Union[str, pathlib.PurePosixPath]
FilePath = Union[str, pathlib.PurePath]


class Repository:
Expand Down Expand Up @@ -127,23 +127,23 @@ def hash(self) -> str:
return make_hash(objects)

@staticmethod
def _pre_process_path(path: Optional[FilePath] = None) -> pathlib.PurePosixPath:
"""Validate and convert the path to instance of ``pathlib.PurePosixPath``.
def _pre_process_path(path: Optional[FilePath] = None) -> pathlib.PurePath:
"""Validate and convert the path to instance of ``pathlib.PurePath``.
This should be called by every method of this class before doing anything, such that it can safely assume that
the path is a ``pathlib.PurePosixPath`` object, which makes path manipulation a lot easier.
the path is a ``pathlib.PurePath`` object, which makes path manipulation a lot easier.
:param path: the path as a ``pathlib.PurePosixPath`` object or `None`.
:raises TypeError: if the type of path was not a str nor a ``pathlib.PurePosixPath`` instance.
:param path: the path as a ``pathlib.PurePath`` object or `None`.
:raises TypeError: if the type of path was not a str nor a ``pathlib.PurePath`` instance.
"""
if path is None:
return pathlib.PurePosixPath()
return pathlib.PurePath()

if isinstance(path, str):
path = pathlib.PurePosixPath(path)
path = pathlib.PurePath(path)

if not isinstance(path, pathlib.PurePosixPath):
raise TypeError('path is not of type `str` nor `pathlib.PurePosixPath`.')
if not isinstance(path, pathlib.PurePath):
raise TypeError('path is not of type `str` nor `pathlib.PurePath`.')

if path.is_absolute():
raise TypeError(f'path `{path}` is not a relative path.')
Expand All @@ -167,7 +167,7 @@ def set_backend(self, backend: AbstractRepositoryBackend) -> None:
type_check(backend, AbstractRepositoryBackend)
self._backend = backend

def _insert_file(self, path: pathlib.PurePosixPath, key: str) -> None:
def _insert_file(self, path: pathlib.PurePath, key: str) -> None:
"""Insert a new file object in the object mapping.
.. note:: this assumes the path is a valid relative path, so should be checked by the caller.
Expand Down Expand Up @@ -334,10 +334,10 @@ def put_object_from_tree(self, filepath: FilePath, path: Optional[FilePath] = No
path = self._pre_process_path(path)

if isinstance(filepath, str):
filepath = pathlib.PurePosixPath(filepath)
filepath = pathlib.PurePath(filepath)

if not isinstance(filepath, pathlib.PurePosixPath):
raise TypeError(f'filepath `{filepath}` is not of type `str` nor `pathlib.PurePosixPath`.')
if not isinstance(filepath, pathlib.PurePath):
raise TypeError(f'filepath `{filepath}` is not of type `str` nor `pathlib.PurePath`.')

if not filepath.is_absolute():
raise TypeError(f'filepath `{filepath}` is not an absolute path.')
Expand All @@ -347,7 +347,7 @@ def put_object_from_tree(self, filepath: FilePath, path: Optional[FilePath] = No
self.create_directory(path)

for root_str, dirnames, filenames in os.walk(filepath):
root = pathlib.PurePosixPath(root_str)
root = pathlib.PurePath(root_str)

for dirname in dirnames:
self.create_directory(path / root.relative_to(filepath) / dirname)
Expand Down Expand Up @@ -457,7 +457,7 @@ def clone(self, source: 'Repository') -> None:
with source.open(root / filename) as handle:
self.put_object_from_filelike(handle, root / filename)

def walk(self, path: Optional[FilePath] = None) -> Iterable[Tuple[pathlib.PurePosixPath, List[str], List[str]]]:
def walk(self, path: Optional[FilePath] = None) -> Iterable[Tuple[pathlib.PurePath, List[str], List[str]]]:
"""Walk over the directories and files contained within this repository.
.. note:: the order of the dirname and filename lists that are returned is not necessarily sorted. This is in
Expand All @@ -466,7 +466,7 @@ def walk(self, path: Optional[FilePath] = None) -> Iterable[Tuple[pathlib.PurePo
:param path: the relative path of the directory within the repository whose contents to walk.
:return: tuples of root, dirnames and filenames just like ``os.walk``, with the exception that the root path is
always relative with respect to the repository root, instead of an absolute path and it is an instance of
``pathlib.PurePosixPath`` instead of a normal string
``pathlib.PurePath`` instead of a normal string
"""
path = self._pre_process_path(path)

Expand Down
Loading

1 comment on commit ce9dcf4

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'pytest-benchmarks:ubuntu-22.04,psql_dos'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: ce9dcf4 Previous: c4dfada Ratio
tests/benchmark/test_json_contains.py::test_deep_json[4-4] 311.48369802220935 iter/sec (stddev: 0.00010126) 674.1841696527913 iter/sec (stddev: 0.00010558) 2.16

This comment was automatically generated by workflow using github-action-benchmark.

CC: @giovannipizzi @agoscinski @GeigerJ2 @khsrali @unkcpz

Please sign in to comment.