Improve legacy artifact remote handlingraoul/1025-legacy-remotes

This creates a new ArtifactRemote class, derived from CASRemote that
extends initialisation to check for an artifact service. This drops the
remote early rather than raising an error on method not found each time
it tries to use it.

Fixes #1025

3 files changed, 45 insertions, 7 deletions

diff --git a/src/buildstream/_artifactcache.py b/src/buildstream/_artifactcache.py
index 91ff762c7..de17ea7ac 100644
--- a/src/buildstream/_artifactcache.py
+++ b/src/buildstream/_artifactcache.py
@@ -25,7 +25,7 @@ from .types import _KeyStrength
 from ._exceptions import ArtifactError, CASError, CASCacheError
 from ._protos.buildstream.v2 import artifact_pb2, artifact_pb2_grpc
 
-from ._cas import CASRemoteSpec
+from ._cas import CASRemoteSpec, CASRemote
 from .storage._casbaseddirectory import CasBasedDirectory
 from ._artifact import Artifact
 from . import utils
@@ -43,6 +43,38 @@ class ArtifactCacheSpec(CASRemoteSpec):
     pass
 
 
+# ArtifactRemote extends CASRemote to check during initialisation that there is
+# an artifact service
+class ArtifactRemote(CASRemote):
+    def __init__(self, *args):
+        super().__init__(*args)
+        self.artifact_service = None
+
+    def init(self):
+        if not self._initialized:
+            # do default initialisation
+            super().init()
+
+            # Add artifact stub
+            self.artifact_service = artifact_pb2_grpc.ArtifactServiceStub(self.channel)
+
+            # Check whether the server supports newer proto based artifact.
+            try:
+                request = artifact_pb2.ArtifactStatusRequest()
+                if self.instance_name:
+                    request.instance_name = self.instance_name
+                self.artifact_service.ArtifactStatus(request)
+            except grpc.RpcError as e:
+                # Check if this remote has the artifact service
+                if e.code() == grpc.StatusCode.UNIMPLEMENTED:
+                    raise ArtifactError(
+                        "Configured remote does not have the BuildStream "
+                        "ArtifactService. Please check remote configuration.")
+                # Else raise exception with details
+                raise ArtifactError(
+                    "Remote initialisation failed: {}".format(e.details()))
+
+
 # An ArtifactCache manages artifacts.
 #
 # Args:
@@ -54,6 +86,7 @@ class ArtifactCache(BaseCache):
     spec_name = "artifact_cache_specs"
     spec_error = ArtifactError
     config_node_name = "artifacts"
+    remote_class = ArtifactRemote
 
     def __init__(self, context):
         super().__init__(context)
diff --git a/src/buildstream/_basecache.py b/src/buildstream/_basecache.py
index 68654b2a0..ef9414787 100644
--- a/src/buildstream/_basecache.py
+++ b/src/buildstream/_basecache.py
@@ -37,6 +37,7 @@ class BaseCache():
     spec_name = None
     spec_error = None
     config_node_name = None
+    remote_class = CASRemote
 
     def __init__(self, context):
         self.context = context
@@ -163,18 +164,19 @@ class BaseCache():
         q = multiprocessing.Queue()
         for remote_spec in remote_specs:
 
-            error = CASRemote.check_remote(remote_spec, q)
+            error = self.remote_class.check_remote(remote_spec, q)
 
             if error and on_failure:
                 on_failure(remote_spec.url, error)
+                continue
             elif error:
                 raise self.spec_error(error)  # pylint: disable=not-callable
-            else:
-                self._has_fetch_remotes = True
-                if remote_spec.push:
-                    self._has_push_remotes = True
 
-                remotes[remote_spec.url] = CASRemote(remote_spec)
+            self._has_fetch_remotes = True
+            if remote_spec.push:
+                self._has_push_remotes = True
+
+            remotes[remote_spec.url] = self.remote_class(remote_spec)
 
         for project in self.context.get_projects():
             remote_specs = self.global_remote_specs
diff --git a/src/buildstream/_cas/casserver.py b/src/buildstream/_cas/casserver.py
index e3a69c77f..203b2f021 100644
--- a/src/buildstream/_cas/casserver.py
+++ b/src/buildstream/_cas/casserver.py
@@ -480,6 +480,9 @@ class _ArtifactServicer(artifact_pb2_grpc.ArtifactServiceServicer):
 
         return artifact
 
+    def ArtifactStatus(self, request, context):
+        return artifact_pb2.ArtifactStatusResponse()
+
     def _check_directory(self, name, digest, context):
         try:
             directory = remote_execution_pb2.Directory()