Add error handling to sgr cloud load

Add a model for the response from the externals REST endpoint and output/raise
introspection errors if they exist.

Author: mildbyte

splitgraph/cloud/__init__.py

@@ -32,6 +32,7 @@
 from splitgraph.cloud.models import (
     AddExternalCredentialRequest,
     AddExternalRepositoriesRequest,
+    AddExternalRepositoriesResponse,
     AddExternalRepositoryRequest,
     ExportJobStatus,
     ExternalResponse,
@@ -75,6 +76,7 @@
 from splitgraph.config.management import patch_and_save_config
 from splitgraph.exceptions import (
     AuthAPIError,
+    DataSourceError,
     GQLAPIError,
     GQLRepoDoesntExistError,
     GQLUnauthenticatedError,
@@ -535,17 +537,33 @@ def bulk_upsert_external(
         self,
         repositories: List[AddExternalRepositoryRequest],
         introspection_mode: IntrospectionMode = IntrospectionMode.EMPTY,
-    ):
+        raise_errors: bool = False,
+    ) -> None:
         request = AddExternalRepositoriesRequest(
             repositories=repositories, introspection_mode=introspection_mode
         )
-        self._perform_request(
+        response = self._perform_request(
             "/bulk-add",
             self.access_token,
             request,
             endpoint=self.externals_endpoint,
             jsonschema_endpoint=True,
+            response_class=AddExternalRepositoriesResponse,
         )
+        assert response
+        if response.errors:
+            for repo_errors in response.errors:
+                for error in repo_errors.errors:
+                    logging.warning(
+                        "Error adding table %s/%s/%s: %s (%s)",
+                        repo_errors.namespace,
+                        repo_errors.repository,
+                        error.table_name,
+                        error.error,
+                        error.error_text,
+                    )
+            if raise_errors:
+                raise DataSourceError("Error introspecting some tables!")
 
 
 def AuthAPIClient(*args, **kwargs):

splitgraph/cloud/models.py

@@ -16,7 +16,7 @@
     Source,
     Table,
 )
-from splitgraph.core.types import Params, TableSchema
+from splitgraph.core.types import MountError, Params, TableSchema
 
 
 class Plugin(BaseModel):
@@ -301,3 +301,13 @@ def from_external(
 class AddExternalRepositoriesRequest(BaseModel):
     repositories: List[AddExternalRepositoryRequest]
     introspection_mode: IntrospectionMode = IntrospectionMode.EMPTY
+
+
+class AddExternalRepositoriesResponse(BaseModel):
+    class RepositoryMountError(BaseModel):
+        namespace: str
+        repository: str
+        errors: List[MountError]
+
+    live_image_hashes: List[Optional[str]]
+    errors: Optional[List[RepositoryMountError]] = None

splitgraph/commandline/cloud.py

@@ -620,6 +620,11 @@ def dump_c(remote, readme_dir, repositories_file, limit_repositories):
     help="Whether to reintrospect tables. none: never reintrospect. all: reintrospect all tables. "
     "empty: only reintrospect tables with an empty schema.",
 )
+@click.option(
+    "--ignore-introspection-errors",
+    is_flag=True,
+    help="If set, will ignore errors when introspecting tables.",
+)
 @click.argument("limit_repositories", type=str, nargs=-1)
 def load_c(
     remote,
@@ -629,6 +634,7 @@ def load_c(
     repositories_file,
     limit_repositories,
     introspection_mode,
+    ignore_introspection_errors,
 ):
     """
     Load a Splitgraph catalog from a YAML file.
@@ -680,7 +686,9 @@ def load_c(
                 )
                 external_repositories.append(external_repository)
         rest_client.bulk_upsert_external(
-            repositories=external_repositories, introspection_mode=introspection_mode
+            repositories=external_repositories,
+            introspection_mode=introspection_mode,
+            raise_errors=not ignore_introspection_errors,
         )
         logging.info(f"Uploaded images for {pluralise('repository', len(external_repositories))}")
 

test/splitgraph/commandline/http_fixtures.py

@@ -399,7 +399,7 @@ def add_external_credential(request, uri, response_headers):
     ]
 
 
-def add_external_repo(initial_private=False):
+def add_external_repo(initial_private=False, error=False):
     def cb(request, uri, response_headers):
         data = json.loads(request.body)
 
@@ -451,7 +451,26 @@ def cb(request, uri, response_headers):
         return [
             200,
             response_headers,
-            json.dumps({"live_image_hashes": ["abcdef12" * 8, "ghijkl34" * 8, "mnoprs56" * 8]}),
+            json.dumps(
+                {
+                    "live_image_hashes": ["abcdef12" * 8, "ghijkl34" * 8, "mnoprs56" * 8],
+                    "errors": [
+                        {
+                            "namespace": "otheruser",
+                            "repository": "somerepo_2",
+                            "errors": [
+                                {
+                                    "table_name": "table_1",
+                                    "error": "SomeError",
+                                    "error_text": "Something bad happened",
+                                }
+                            ],
+                        }
+                    ]
+                    if not error
+                    else [],
+                }
+            ),
         ]
 
     return cb

test/splitgraph/commandline/test_cloud_metadata.py

@@ -273,8 +273,9 @@ def test_commandline_dump(snapshot):
 
 
 @pytest.mark.parametrize("initial_private", [True, False])
+@pytest.mark.parametrize("errors", [True, False])
 @httpretty.activate(allow_net_connect=False)
-def test_commandline_load(initial_private):
+def test_commandline_load(initial_private, errors):
     runner = CliRunner()
 
     httpretty.register_uri(
@@ -326,6 +327,7 @@ def get_remote_param(remote, param):
                 os.path.join(RESOURCES, "splitgraph_yml", "readmes"),
                 "-f",
                 os.path.join(RESOURCES, "splitgraph_yml", "splitgraph.yml"),
+                "--ignore-introspection-errors",
             ],
             catch_exceptions=False,
         )
@@ -339,6 +341,12 @@ def get_remote_param(remote, param):
         reqs.pop()  # discard duplicate request
         assert_repository_profiles(reqs.pop())
 
+        if errors:
+            assert (
+                "Error adding table otheruser/somerepo_2/table_1: "
+                "SomeError (Something bad happened)" in result.output
+            )
+
 
 def test_project_validate(snapshot):
     # Use the same file as the merging test