Skip to content

Commit

Permalink
Switch configuration executor List type to Sequence
Browse files Browse the repository at this point in the history
The list of executors in configuration is intended to be read only.
Using Sequence for this makes that more explicit, as Sequences cannot
be modified.

This is part of a move towards using Sequence rather than List in
many places (although the runtime type remains a list) because Sequence
has better behaviour with subclasses of its parameter type: it is
covariant, rather than invariant.

This PR rearranges the validation logic a bit so that there is no
longer an arbitrary property setter for the executor property - it was
always a bug to replace the executor list in a config once parsl has
been configured, and removing this property setter enforces that more
strongly.
  • Loading branch information
benclifford committed Nov 1, 2023
1 parent f8b1836 commit c3afd05
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions parsl/config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import typeguard

from typing import Callable, List, Optional, Sequence, Union
from typing import Callable, Optional, Sequence, Union
from typing_extensions import Literal

from parsl.utils import RepresentationMixin
Expand All @@ -20,8 +20,8 @@ class Config(RepresentationMixin):
Parameters
----------
executors : list of ParslExecutor, optional
List of `ParslExecutor` instances to use for executing tasks.
executors : sequence of ParslExecutor, optional
List (or other sequence) of `ParslExecutor` instances to use for executing tasks.
Default is [:class:`~parsl.executors.threads.ThreadPoolExecutor()`].
app_cache : bool, optional
Enable app caching. Default is True.
Expand Down Expand Up @@ -73,7 +73,7 @@ class Config(RepresentationMixin):

@typeguard.typechecked
def __init__(self,
executors: Optional[List[ParslExecutor]] = None,
executors: Optional[Sequence[ParslExecutor]] = None,
app_cache: bool = True,
checkpoint_files: Optional[Sequence[str]] = None,
checkpoint_mode: Union[None,
Expand All @@ -92,9 +92,12 @@ def __init__(self,
monitoring: Optional[MonitoringHub] = None,
usage_tracking: bool = False,
initialize_logging: bool = True) -> None:
self._executors: Sequence[ParslExecutor]
if executors is None:
executors = [ThreadPoolExecutor()]
self.executors = executors
self._executors = [ThreadPoolExecutor()]
else:
self._validate_executors(executors)
self._executors = executors
self.app_cache = app_cache
self.checkpoint_files = checkpoint_files
self.checkpoint_mode = checkpoint_mode
Expand Down Expand Up @@ -125,11 +128,9 @@ def __init__(self,
def executors(self) -> Sequence[ParslExecutor]:
return self._executors

@executors.setter
def executors(self, executors: Sequence[ParslExecutor]):
def _validate_executors(self, executors: Sequence[ParslExecutor]) -> None:
labels = [e.label for e in executors]
duplicates = [e for n, e in enumerate(labels) if e in labels[:n]]
if len(duplicates) > 0:
raise ConfigurationError('Executors must have unique labels ({})'.format(
', '.join(['label={}'.format(repr(d)) for d in duplicates])))
self._executors = executors

0 comments on commit c3afd05

Please sign in to comment.