[ISSUE #19733] code review and adding validation for spec file as well

airbyte-cdk/python/airbyte_cdk/connector.py

@@ -54,7 +54,7 @@ def read_json_file(config_path: str) -> TConfig:
         try:
             return json.loads(contents)
         except json.JSONDecodeError as error:
-            raise ValueError(f"Could not read json file: `{error}`")
+            raise ValueError(f"Could not read json file {config_path}: {error}. Please ensure that it is a valid JSON.")
 
     @staticmethod
     def write_config(config: TConfig, config_path: str):
@@ -78,7 +78,10 @@ def spec(self, logger: logging.Logger) -> ConnectorSpecification:
         if yaml_spec:
             spec_obj = yaml.load(yaml_spec, Loader=yaml.SafeLoader)
         elif json_spec:
-            spec_obj = json.loads(json_spec)
+            try:
+                spec_obj = json.loads(json_spec)
+            except json.JSONDecodeError as error:
+                raise ValueError(f"Could not read json spec file: {error}. Please ensure that it is a valid JSON.")
         else:
             raise FileNotFoundError("Unable to find spec.yaml or spec.json in the package.")
 

airbyte-cdk/python/unit_tests/test_connector.py

@@ -115,6 +115,14 @@ def use_json_spec(self):
         yield
         os.remove(json_path)
 
+    @pytest.fixture
+    def use_invalid_json_spec(self):
+        json_path = os.path.join(SPEC_ROOT, "spec.json")
+        with open(json_path, "w") as f:
+            f.write("the content of this file is not JSON")
+        yield
+        os.remove(json_path)
+
     @pytest.fixture
     def use_yaml_spec(self):
         spec = {"documentationUrl": "https://airbyte.com/#yaml", "connectionSpecification": self.CONNECTION_SPECIFICATION}
@@ -130,6 +138,10 @@ def test_spec_from_json_file(self, integration, use_json_spec):
         assert connector_spec.documentationUrl == "https://airbyte.com/#json"
         assert connector_spec.connectionSpecification == self.CONNECTION_SPECIFICATION
 
+    def test_spec_from_improperly_formatted_json_file(self, integration, use_invalid_json_spec):
+        with pytest.raises(ValueError, match="Could not read json spec file"):
+            integration.spec(logger)
+
     def test_spec_from_yaml_file(self, integration, use_yaml_spec):
         connector_spec = integration.spec(logger)
         assert connector_spec.documentationUrl == "https://airbyte.com/#yaml"

airbyte-integrations/connectors/source-s3/source_s3/source.py

@@ -57,8 +57,8 @@ class SourceS3(SourceFilesAbstract):
     spec_class = SourceS3Spec
     documentation_url = "https://docs.airbyte.com/integrations/sources/s3"
 
-    def read_config(self, config_path: str) -> Mapping[str, Any]:
-        config: Mapping[str, Any] = super().read_config(config_path)
+    def read_json_file(self, config_path: str) -> Mapping[str, Any]:
+        config: Mapping[str, Any] = super().read_json_file(config_path)
         if config.get("format", {}).get("delimiter") == r"\t":
             config["format"]["delimiter"] = "\t"
         return config

airbyte-integrations/connectors/source-s3/unit_tests/conftest.py

@@ -49,7 +49,7 @@ def config_fixture(tmp_path):
             fp,
         )
     source = SourceS3()
-    config = source.read_config(config_file)
+    config = source.read_json_file(config_file)
     return config
 
 

airbyte-integrations/connectors/source-s3/unit_tests/test_source.py

@@ -19,7 +19,7 @@ def test_transform_backslash_t_to_tab(tmp_path):
     with open(config_file, "w") as fp:
         json.dump({"format": {"delimiter": "\\t"}}, fp)
     source = SourceS3()
-    config = source.read_config(config_file)
+    config = source.read_json_file(config_file)
     assert config["format"]["delimiter"] == "\t"