Remove cache-specifc spec classes

This was almost entirely just historical code duplication.

8 files changed, 32 insertions, 97 deletions

diff --git a/src/buildstream/_artifactcache.py b/src/buildstream/_artifactcache.py
index 3357f986a..73047d376 100644
--- a/src/buildstream/_artifactcache.py
+++ b/src/buildstream/_artifactcache.py
@@ -25,22 +25,12 @@ from ._exceptions import ArtifactError, CASError, CASCacheError, CASRemoteError
 from ._protos.buildstream.v2 import buildstream_pb2, buildstream_pb2_grpc, \
     artifact_pb2, artifact_pb2_grpc
 
-from ._cas import CASRemoteSpec, CASRemote
+from ._cas import CASRemote
 from .storage._casbaseddirectory import CasBasedDirectory
 from ._artifact import Artifact
 from . import utils
 
 
-# An ArtifactCacheSpec holds the user configuration for a single remote
-# artifact cache.
-#
-# Args:
-#     url (str): Location of the remote artifact cache
-#     push (bool): Whether we should attempt to push artifacts to this cache,
-#                  in addition to pulling from it.
-#
-class ArtifactCacheSpec(CASRemoteSpec):
-    pass
 
 
 # ArtifactRemote extends CASRemote to check during initialisation that there is
@@ -86,7 +76,6 @@ class ArtifactRemote(CASRemote):
 #
 class ArtifactCache(BaseCache):
 
-    spec_class = ArtifactCacheSpec
     spec_name = "artifact_cache_specs"
     spec_error = ArtifactError
     config_node_name = "artifacts"
diff --git a/src/buildstream/_basecache.py b/src/buildstream/_basecache.py
index 1f306a808..faf24e972 100644
--- a/src/buildstream/_basecache.py
+++ b/src/buildstream/_basecache.py
@@ -26,11 +26,11 @@ from . import _yaml
 from ._cas import CASRemote
 from ._message import Message, MessageType
 from ._exceptions import LoadError
+from ._remote import RemoteSpec
 
 if TYPE_CHECKING:
     from typing import Optional, Type
     from ._exceptions import BstError
-    from ._cas import CASRemoteSpec
 
 
 # Base Cache for Caches to derive from
@@ -39,11 +39,10 @@ class BaseCache():
 
     # None of these should ever be called in the base class, but this appeases
     # pylint to some degree
-    spec_class = None           # type: Type[CASRemoteSpec]
-    spec_name = None            # type: str
-    spec_error = None           # type: Type[BstError]
-    config_node_name = None     # type: str
-    remote_class = CASRemote    # type: Type[CASRemote]
+    spec_name = None           # type: Type[RemoteSpec]
+    spec_error = None          # type: Type[BstError]
+    config_node_name = None    # type: str
+    remote_class = CASRemote   # type: Type[CASRemote]
 
     def __init__(self, context):
         self.context = context
@@ -90,7 +89,7 @@ class BaseCache():
     #   basedir (str): The base directory for relative paths
     #
     # Returns:
-    #   A list of ArtifactCacheSpec instances.
+    #   A list of RemoteSpec instances.
     #
     # Raises:
     #   LoadError, if the config block contains invalid keys.
@@ -110,7 +109,7 @@ class BaseCache():
                                       .format(provenance, cls.config_node_name), _yaml.LoadErrorReason.INVALID_DATA)
 
         for spec_node in artifacts:
-            cache_specs.append(cls.spec_class._new_from_config_node(spec_node, basedir))
+            cache_specs.append(RemoteSpec.new_from_config_node(spec_node))
 
         return cache_specs
 
@@ -124,7 +123,7 @@ class BaseCache():
     #     project (Project): The BuildStream project
     #
     # Returns:
-    #   A list of ArtifactCacheSpec instances describing the remote artifact caches.
+    #   A list of RemoteSpec instances describing the remote caches.
     #
     @classmethod
     def _configured_remote_cache_specs(cls, context, project):
@@ -159,12 +158,12 @@ class BaseCache():
         has_remote_caches = False
         if remote_url:
             # pylint: disable=not-callable
-            self._set_remotes([self.spec_class(remote_url, push=True)])
+            self._set_remotes([RemoteSpec(remote_url, push=True)])
             has_remote_caches = True
         if use_config:
             for project in self.context.get_projects():
                 caches = self._configured_remote_cache_specs(self.context, project)
-                if caches:  # caches is a list of spec_class instances
+                if caches:  # caches is a list of RemoteSpec instances
                     self._set_remotes(caches, project=project)
                     has_remote_caches = True
         if has_remote_caches:
diff --git a/src/buildstream/_cas/__init__.py b/src/buildstream/_cas/__init__.py
index a88e41371..17e3a3fd9 100644
--- a/src/buildstream/_cas/__init__.py
+++ b/src/buildstream/_cas/__init__.py
@@ -18,4 +18,4 @@
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 
 from .cascache import CASCache
-from .casremote import CASRemote, CASRemoteSpec
+from .casremote import CASRemote
diff --git a/src/buildstream/_cas/casremote.py b/src/buildstream/_cas/casremote.py
index ba0477550..35bbb68ec 100644
--- a/src/buildstream/_cas/casremote.py
+++ b/src/buildstream/_cas/casremote.py
@@ -20,52 +20,6 @@ from .. import utils
 _MAX_PAYLOAD_BYTES = 1024 * 1024
 
 
-class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key client_cert instance_name')):
-
-    # _new_from_config_node
-    #
-    # Creates an CASRemoteSpec() from a YAML loaded node
-    #
-    @staticmethod
-    def _new_from_config_node(spec_node, basedir=None):
-        spec_node.validate_keys(['url', 'push', 'server-cert', 'client-key', 'client-cert', 'instance-name'])
-        url = spec_node.get_str('url')
-        push = spec_node.get_bool('push', default=False)
-        if not url:
-            provenance = spec_node.get_node('url').get_provenance()
-            raise LoadError("{}: empty artifact cache URL".format(provenance), LoadErrorReason.INVALID_DATA)
-
-        instance_name = spec_node.get_str('instance-name', default=None)
-
-        server_cert = spec_node.get_str('server-cert', default=None)
-        if server_cert and basedir:
-            server_cert = os.path.join(basedir, server_cert)
-
-        client_key = spec_node.get_str('client-key', default=None)
-        if client_key and basedir:
-            client_key = os.path.join(basedir, client_key)
-
-        client_cert = spec_node.get_str('client-cert', default=None)
-        if client_cert and basedir:
-            client_cert = os.path.join(basedir, client_cert)
-
-        if client_key and not client_cert:
-            provenance = spec_node.get_node('client-key').get_provenance()
-            raise LoadError("{}: 'client-key' was specified without 'client-cert'".format(provenance),
-                            LoadErrorReason.INVALID_DATA)
-
-        if client_cert and not client_key:
-            provenance = spec_node.get_node('client-cert').get_provenance()
-            raise LoadError("{}: 'client-cert' was specified without 'client-key'".format(provenance),
-                            LoadErrorReason.INVALID_DATA)
-
-        return CASRemoteSpec(url, push, server_cert, client_key, client_cert, instance_name)
-
-
-# Disable type-checking since "Callable[...] has no attributes __defaults__"
-CASRemoteSpec.__new__.__defaults__ = (None, None, None, None)   # type: ignore
-
-
 class BlobNotFound(CASRemoteError):
 
     def __init__(self, blob, msg):
diff --git a/src/buildstream/_remote.py b/src/buildstream/_remote.py
index 087b62dd6..9761e8238 100644
--- a/src/buildstream/_remote.py
+++ b/src/buildstream/_remote.py
@@ -25,7 +25,7 @@ import grpc
 
 from . import _signals
 from . import utils
-from ._exceptions import RemoteError, LoadError, LoadErrorReason, ImplError
+from ._exceptions import LoadError, LoadErrorReason, ImplError
 from ._protos.google.bytestream import bytestream_pb2_grpc
 
 
@@ -88,6 +88,8 @@ class RemoteSpec(namedtuple('RemoteSpec', 'url push server_cert client_key clien
 #
 # Note that defaults are specified from the right, and ommitted values
 # are considered mandatory.
+#
+# Disable type-checking since "Callable[...] has no attributes __defaults__"
 RemoteSpec.__new__.__defaults__ = (
     # mandatory          # url            - The url of the remote
     # mandatory          # push           - Whether the remote should be used for pushing
@@ -95,7 +97,7 @@ RemoteSpec.__new__.__defaults__ = (
     None,                # client_key     - The (private) client key
     None,                # client_cert    - The (public) client certificate
     None                 # instance_name  - The (grpc) instance name of the remote
-)
+)   # type: ignore
 
 
 # BaseRemote():
diff --git a/src/buildstream/_sourcecache.py b/src/buildstream/_sourcecache.py
index 64498ba32..2a6a6e220 100644
--- a/src/buildstream/_sourcecache.py
+++ b/src/buildstream/_sourcecache.py
@@ -20,7 +20,7 @@
 import os
 import grpc
 
-from ._cas import CASRemote, CASRemoteSpec
+from ._cas import CASRemote
 from .storage._casbaseddirectory import CasBasedDirectory
 from ._basecache import BaseCache
 from ._exceptions import CASError, CASRemoteError, SourceCacheError
@@ -29,16 +29,6 @@ from ._protos.buildstream.v2 import buildstream_pb2, buildstream_pb2_grpc, \
     source_pb2, source_pb2_grpc
 
 
-# Holds configuration for a remote used for the source cache.
-#
-# Args:
-#     url (str): Location of the remote source cache
-#     push (bool): Whether we should attempt to push sources to this cache,
-#                  in addition to pulling from it.
-#     instance-name (str): Name if any, of instance of server
-#
-class SourceCacheSpec(CASRemoteSpec):
-    pass
 
 
 class SourceRemote(CASRemote):
@@ -85,7 +75,6 @@ class SourceRemote(CASRemote):
 #
 class SourceCache(BaseCache):
 
-    spec_class = SourceCacheSpec
     spec_name = "source_cache_specs"
     spec_error = SourceCacheError
     config_node_name = "source-caches"
diff --git a/src/buildstream/sandbox/_sandboxremote.py b/src/buildstream/sandbox/_sandboxremote.py
index cf034dbf9..678b11c32 100644
--- a/src/buildstream/sandbox/_sandboxremote.py
+++ b/src/buildstream/sandbox/_sandboxremote.py
@@ -37,7 +37,8 @@ from .._protos.google.rpc import code_pb2
 from .._exceptions import BstError, SandboxError
 from .. import _yaml
 from .._protos.google.longrunning import operations_pb2, operations_pb2_grpc
-from .._cas import CASRemote, CASRemoteSpec
+from .._cas import CASRemote
+from .._remote import RemoteSpec
 
 
 class RemoteExecutionSpec(namedtuple('RemoteExecutionSpec', 'exec_service storage_service action_service')):
@@ -98,11 +99,11 @@ class SandboxRemote(Sandbox):
         self.exec_instance = config.exec_service.get('instance-name', None)
         self.storage_instance = config.storage_service.get('instance-name', None)
 
-        self.storage_remote_spec = CASRemoteSpec(self.storage_url, push=True,
-                                                 server_cert=config.storage_service.get('server-cert'),
-                                                 client_key=config.storage_service.get('client-key'),
-                                                 client_cert=config.storage_service.get('client-cert'),
-                                                 instance_name=self.storage_instance)
+        self.storage_remote_spec = RemoteSpec(self.storage_url, push=True,
+                                              server_cert=config.storage_service.get('server-cert'),
+                                              client_key=config.storage_service.get('client-key'),
+                                              client_cert=config.storage_service.get('client-cert'),
+                                              instance_name=self.storage_instance)
         self.operation_name = None
 
     def info(self, msg):
diff --git a/tests/artifactcache/config.py b/tests/artifactcache/config.py
index 08d6f74bb..2f235f38c 100644
--- a/tests/artifactcache/config.py
+++ b/tests/artifactcache/config.py
@@ -6,7 +6,8 @@ import os
 
 import pytest
 
-from buildstream._artifactcache import ArtifactCacheSpec, ArtifactCache
+from buildstream._remote import RemoteSpec
+from buildstream._artifactcache import ArtifactCache
 from buildstream._project import Project
 from buildstream.utils import _deduplicate
 from buildstream import _yaml
@@ -18,12 +19,12 @@ from tests.testutils import dummy_context
 
 
 DATA_DIR = os.path.dirname(os.path.realpath(__file__))
-cache1 = ArtifactCacheSpec(url='https://example.com/cache1', push=True)
-cache2 = ArtifactCacheSpec(url='https://example.com/cache2', push=False)
-cache3 = ArtifactCacheSpec(url='https://example.com/cache3', push=False)
-cache4 = ArtifactCacheSpec(url='https://example.com/cache4', push=False)
-cache5 = ArtifactCacheSpec(url='https://example.com/cache5', push=False)
-cache6 = ArtifactCacheSpec(url='https://example.com/cache6', push=True)
+cache1 = RemoteSpec(url='https://example.com/cache1', push=True)
+cache2 = RemoteSpec(url='https://example.com/cache2', push=False)
+cache3 = RemoteSpec(url='https://example.com/cache3', push=False)
+cache4 = RemoteSpec(url='https://example.com/cache4', push=False)
+cache5 = RemoteSpec(url='https://example.com/cache5', push=False)
+cache6 = RemoteSpec(url='https://example.com/cache6', push=True)
 
 
 # Generate cache configuration fragments for the user config and project config files.