Skip to content

Commit

Permalink
[ISSUE #19733] remove breaking change
Browse files Browse the repository at this point in the history
  • Loading branch information
maxi297 committed Dec 6, 2022
1 parent 40e08b2 commit b9c933e
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 22 deletions.
3 changes: 3 additions & 0 deletions airbyte-cdk/python/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## 0.12.1
Revert breaking change on `read_config` while keeping the improvement on the error message

## 0.12.0
Improve error readability when reading JSON config files

Expand Down
18 changes: 14 additions & 4 deletions airbyte-cdk/python/airbyte_cdk/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import os
import pkgutil
from abc import ABC, abstractmethod
from typing import Any, Generic, Mapping, Optional, Protocol, TypeVar
from typing import Any, Generic, List, Mapping, Optional, Protocol, TypeVar, Union

import yaml
from airbyte_cdk.models import AirbyteConnectionStatus, ConnectorSpecification
Expand Down Expand Up @@ -47,14 +47,24 @@ def configure(self, config: Mapping[str, Any], temp_dir: str) -> TConfig:
"""

@staticmethod
def read_json_file(config_path: str) -> Mapping[str, Any]:
with open(config_path, "r") as file:
def read_config(config_path: str) -> Mapping[str, Any]:
config = BaseConnector._read_json_file(config_path)
if isinstance(config, Mapping):
return config
else:
raise ValueError(
f"The content of {config_path} is not an object and therefore is not a valid config. Please ensure the file represent a config."
)

@staticmethod
def _read_json_file(file_path: str) -> Union[None, bool, float, int, str, List[Any], Mapping[str, Any]]:
with open(file_path, "r") as file:
contents = file.read()

try:
return json.loads(contents)
except json.JSONDecodeError as error:
raise ValueError(f"Could not read json file {config_path}: {error}. Please ensure that it is a valid JSON.")
raise ValueError(f"Could not read json file {file_path}: {error}. Please ensure that it is a valid JSON.")

@staticmethod
def write_config(config: TConfig, config_path: str):
Expand Down
2 changes: 1 addition & 1 deletion airbyte-cdk/python/airbyte_cdk/destinations/destination.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file(config_path=parsed_args.config)
config = self.read_config(config_path=parsed_args.config)
if self.check_config_against_spec or cmd == "check":
try:
check_config_against_spec_or_exit(config, spec)
Expand Down
2 changes: 1 addition & 1 deletion airbyte-cdk/python/airbyte_cdk/entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file(parsed_args.config)
raw_config = self.source.read_config(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
Expand Down
2 changes: 1 addition & 1 deletion airbyte-cdk/python/airbyte_cdk/sources/singer/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file(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)
Expand Down
4 changes: 2 additions & 2 deletions airbyte-cdk/python/airbyte_cdk/sources/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,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 = self.read_json_file(state_path)
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)
Expand Down Expand Up @@ -86,4 +86,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_json_file(catalog_path))
return ConfiguredAirbyteCatalog.parse_obj(self._read_json_file(catalog_path))
2 changes: 1 addition & 1 deletion airbyte-cdk/python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

setup(
name="airbyte-cdk",
version="0.12.0",
version="0.12.1",
description="A framework for writing Airbyte Connectors.",
long_description=README,
long_description_content_type="text/markdown",
Expand Down
8 changes: 4 additions & 4 deletions airbyte-cdk/python/unit_tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ def integration():
return MockConnector()


def test_read_json_file(nonempty_file, integration: Connector, mock_config):
actual = integration.read_json_file(nonempty_file.name)
def test_read_config(nonempty_file, integration: Connector, mock_config):
actual = integration.read_config(nonempty_file.name)
assert mock_config == actual


def test_read_non_json_file(nonjson_file, integration: Connector):
def test_read_non_json_config(nonjson_file, integration: Connector):
with pytest.raises(ValueError, match="Could not read json file"):
integration.read_json_file(nonjson_file.name)
integration.read_config(nonjson_file.name)


def test_write_config(integration, mock_config):
Expand Down
2 changes: 1 addition & 1 deletion airbyte-cdk/python/unit_tests/test_entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file", return_value=config)
mocker.patch.object(MockSource, "read_config", return_value=config)
mocker.patch.object(MockSource, "configure", return_value=config)
return config

Expand Down
6 changes: 3 additions & 3 deletions airbyte-cdk/python/unit_tests/test_secure_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file", return_value=None)
mocker.patch.object(MockSource, "read_config", return_value=None)
mocker.patch.object(MockSource, "read_state", return_value={})
mocker.patch.object(MockSource, "read_catalog", return_value={})
list(entrypoint.run(parsed_args))
Expand Down Expand Up @@ -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_json_file", return_value=None)
mocker.patch.object(MockSource, "read_config", return_value=None)
mocker.patch.object(MockSource, "read_state", return_value={})
mocker.patch.object(MockSource, "read_catalog", return_value={})

Expand Down Expand Up @@ -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_json_file", return_value=None)
mocker.patch.object(MockSource, "read_config", 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))
Expand Down
4 changes: 2 additions & 2 deletions airbyte-integrations/connectors/source-s3/source_s3/source.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class SourceS3(SourceFilesAbstract):
spec_class = SourceS3Spec
documentation_url = "https://docs.airbyte.com/integrations/sources/s3"

def read_json_file(self, config_path: str) -> Mapping[str, Any]:
config: Mapping[str, Any] = super().read_json_file(config_path)
def read_config(self, config_path: str) -> Mapping[str, Any]:
config: Mapping[str, Any] = super().read_config(config_path)
if config.get("format", {}).get("delimiter") == r"\t":
config["format"]["delimiter"] = "\t"
return config
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def config_fixture(tmp_path):
fp,
)
source = SourceS3()
config = source.read_json_file(config_file)
config = source.read_config(config_file)
return config


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_json_file(config_file)
config = source.read_config(config_file)
assert config["format"]["delimiter"] == "\t"


Expand Down

0 comments on commit b9c933e

Please sign in to comment.