From c3afd050bdb9df93f87ad29571e948bc02d4b8f5 Mon Sep 17 00:00:00 2001
From: Ben Clifford <benc@hawaga.org.uk>
Date: Wed, 10 May 2023 05:48:15 +0000
Subject: [PATCH] Switch configuration executor List type to Sequence

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.
---
 parsl/config.py | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/parsl/config.py b/parsl/config.py
index 0f56c5b9cb..b924c32bd4 100644
--- a/parsl/config.py
+++ b/parsl/config.py
@@ -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
@@ -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.
@@ -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,
@@ -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
@@ -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