[ISSUE airbytehq#19733] clarify error message when reading config files

airbyte-cdk/python/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 0.12.0
+Improve error readability when error while reading JSON config files
+
 ## 0.11.1
 Low-code: Fix the component manifest schema to and validate check instead of checker
 

airbyte-cdk/python/airbyte_cdk/connector.py

@@ -47,10 +47,14 @@ def configure(self, config: Mapping[str, Any], temp_dir: str) -> TConfig:
         """
 
     @staticmethod
-    def read_config(config_path: str) -> TConfig:
+    def read_json_file(config_path: str) -> TConfig:
         with open(config_path, "r") as file:
             contents = file.read()
-        return json.loads(contents)
+
+        try:
+            return json.loads(contents)
+        except json.JSONDecodeError as error:
+            raise ValueError(f"Could not read json file {config_path}: `{error}`")
 
     @staticmethod
     def write_config(config: TConfig, config_path: str):

airbyte-cdk/python/airbyte_cdk/destinations/destination.py

@@ -94,7 +94,7 @@ def run_cmd(self, parsed_args: argparse.Namespace) -> Iterable[AirbyteMessage]:
         if cmd == "spec":
             yield AirbyteMessage(type=Type.SPEC, spec=spec)
             return
-        config = self.read_config(config_path=parsed_args.config)
+        config = self.read_json_file(config_path=parsed_args.config)
         if self.check_config_against_spec or cmd == "check":
             try:
                 check_config_against_spec_or_exit(config, spec)

airbyte-cdk/python/airbyte_cdk/entrypoint.py

@@ -82,7 +82,7 @@ def run(self, parsed_args: argparse.Namespace) -> Iterable[str]:
                 message = AirbyteMessage(type=Type.SPEC, spec=source_spec)
                 yield message.json(exclude_unset=True)
             else:
-                raw_config = self.source.read_config(parsed_args.config)
+                raw_config = self.source.read_json_file(parsed_args.config)
                 config = self.source.configure(raw_config, temp_dir)
 
                 # Now that we have the config, we can use it to get a list of ai airbyte_secrets

airbyte-cdk/python/airbyte_cdk/sources/singer/source.py

@@ -98,7 +98,7 @@ def read(self, logger: logging.Logger, config: ConfigContainer, catalog_path: st
         Implements the parent class read method.
         """
         catalogs = self._discover_internal(logger, config.config_path)
-        masked_airbyte_catalog = ConfiguredAirbyteCatalog.parse_obj(self.read_config(catalog_path))
+        masked_airbyte_catalog = ConfiguredAirbyteCatalog.parse_obj(self.read_json_file(catalog_path))
         selected_singer_catalog_path = SingerHelper.create_singer_catalog_with_selection(masked_airbyte_catalog, catalogs.singer_catalog)
 
         read_cmd = self.read_cmd(logger, config.config_path, selected_singer_catalog_path, state_path)

airbyte-cdk/python/airbyte_cdk/sources/source.py

@@ -54,7 +54,7 @@ def read_state(self, state_path: str) -> Union[List[AirbyteStateMessage], Mutabl
         :return: The complete stream state based on the connector's previous sync
         """
         if state_path:
-            state_obj = json.loads(open(state_path, "r").read())
+            state_obj = self.read_json_file(state_path)
             if not state_obj:
                 return self._emit_legacy_state_format({})
             is_per_stream_state = isinstance(state_obj, List)
@@ -87,4 +87,4 @@ def _emit_legacy_state_format(self, state_obj) -> Union[List[AirbyteStateMessage
 
     # can be overridden to change an input catalog
     def read_catalog(self, catalog_path: str) -> ConfiguredAirbyteCatalog:
-        return ConfiguredAirbyteCatalog.parse_obj(self.read_config(catalog_path))
+        return ConfiguredAirbyteCatalog.parse_obj(self.read_json_file(catalog_path))

airbyte-cdk/python/setup.py

@@ -15,7 +15,7 @@
 
 setup(
     name="airbyte-cdk",
-    version="0.11.1",
+    version="0.12.0",
     description="A framework for writing Airbyte Connectors.",
     long_description=README,
     long_description_content_type="text/markdown",

airbyte-cdk/python/unit_tests/test_connector.py

@@ -65,16 +65,29 @@ def nonempty_file(mock_config):
         yield file
 
 
+@pytest.fixture
+def nonjson_file(mock_config):
+    with tempfile.NamedTemporaryFile("w") as file:
+        file.write("the content of this file is not JSON")
+        file.flush()
+        yield file
+
+
 @pytest.fixture
 def integration():
     return MockConnector()
 
 
-def test_read_config(nonempty_file, integration: Connector, mock_config):
-    actual = integration.read_config(nonempty_file.name)
+def test_read_json_file(nonempty_file, integration: Connector, mock_config):
+    actual = integration.read_json_file(nonempty_file.name)
     assert mock_config == actual
 
 
+def test_read_non_json_file(nonjson_file, integration: Connector):
+    with pytest.raises(ValueError, match="Could not read json file"):
+        integration.read_json_file(nonjson_file.name)
+
+
 def test_write_config(integration, mock_config):
     config_path = Path(tempfile.gettempdir()) / "config.json"
     integration.write_config(mock_config, str(config_path))

airbyte-cdk/python/unit_tests/test_entrypoint.py

@@ -130,7 +130,7 @@ def test_run_spec(entrypoint: AirbyteEntrypoint, mocker):
 @pytest.fixture
 def config_mock(mocker, request):
     config = request.param if hasattr(request, "param") else {"username": "fake"}
-    mocker.patch.object(MockSource, "read_config", return_value=config)
+    mocker.patch.object(MockSource, "read_json_file", return_value=config)
     mocker.patch.object(MockSource, "configure", return_value=config)
     return config
 

airbyte-cdk/python/unit_tests/test_secure_logger.py

@@ -134,7 +134,7 @@ def test_airbyte_secret_is_masked_on_logger_output(source_spec, mocker, config,
         return_value=ConnectorSpecification(connectionSpecification=source_spec),
     )
     mocker.patch.object(MockSource, "configure", return_value=config)
-    mocker.patch.object(MockSource, "read_config", return_value=None)
+    mocker.patch.object(MockSource, "read_json_file", return_value=None)
     mocker.patch.object(MockSource, "read_state", return_value={})
     mocker.patch.object(MockSource, "read_catalog", return_value={})
     list(entrypoint.run(parsed_args))
@@ -180,7 +180,7 @@ def read(
         return_value=ConnectorSpecification(connectionSpecification=source_spec),
     )
     mocker.patch.object(MockSource, "configure", return_value=simple_config)
-    mocker.patch.object(MockSource, "read_config", return_value=None)
+    mocker.patch.object(MockSource, "read_json_file", return_value=None)
     mocker.patch.object(MockSource, "read_state", return_value={})
     mocker.patch.object(MockSource, "read_catalog", return_value={})
 
@@ -227,7 +227,7 @@ def read(
         return_value=ConnectorSpecification(connectionSpecification=source_spec),
     )
     mocker.patch.object(MockSource, "configure", return_value=simple_config)
-    mocker.patch.object(MockSource, "read_config", return_value=None)
+    mocker.patch.object(MockSource, "read_json_file", return_value=None)
     mocker.patch.object(MockSource, "read_state", return_value={})
     mocker.patch.object(MockSource, "read_catalog", return_value={})
     mocker.patch.object(MockSource, "read", side_effect=Exception("Exception:" + NOT_A_SECRET_VALUE))