From d9648f57326a109fe463364dc64247b3da068783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez-Mondrag=C3=B3n?= Date: Fri, 29 Nov 2024 22:39:40 -0600 Subject: [PATCH 1/3] fix: Ensure the required global folder tap settings are merged into the concrete implementation settings --- singer_sdk/contrib/filesystem/tap.py | 15 +++++++++++---- singer_sdk/plugin_base.py | 13 ++++++------- singer_sdk/tap_base.py | 4 ++-- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/singer_sdk/contrib/filesystem/tap.py b/singer_sdk/contrib/filesystem/tap.py index ff0c98819..6c9579211 100644 --- a/singer_sdk/contrib/filesystem/tap.py +++ b/singer_sdk/contrib/filesystem/tap.py @@ -105,6 +105,8 @@ class FolderTap(Tap, t.Generic[_T]): This should be a subclass of `FileStream`. """ + dynamic_catalog: bool = True + config_jsonschema: t.ClassVar[dict] = {"properties": {}} @classmethod @@ -124,11 +126,16 @@ def append_builtin_config(cls: type[FolderTap], config_jsonschema: dict) -> None config_jsonschema: [description] """ - def _merge_missing(source_jsonschema: dict, target_jsonschema: dict) -> None: + def _merge_missing(src: dict, tgt: dict) -> None: # Append any missing properties in the target with those from source. - for k, v in source_jsonschema["properties"].items(): - if k not in target_jsonschema["properties"]: - target_jsonschema["properties"][k] = v + for k, v in src["properties"].items(): + if k not in tgt["properties"]: + tgt["properties"][k] = v + + # Merge the required fields + source_required = src.get("required", []) + target_required = tgt.get("required", []) + tgt["required"] = list(set(source_required + target_required)) _merge_missing(BASE_CONFIG_SCHEMA, config_jsonschema) diff --git a/singer_sdk/plugin_base.py b/singer_sdk/plugin_base.py index 1b8735f2f..62f72f804 100644 --- a/singer_sdk/plugin_base.py +++ b/singer_sdk/plugin_base.py @@ -160,14 +160,13 @@ def __init__( validate_config: True to require validation of config settings. Raises: - ValueError: If config is not a dict or path string. + TypeError: If config is not a dict or path string. """ - if not config: - config_dict = {} - elif isinstance(config, (str, PurePath)): + config = config or {} + if isinstance(config, (str, PurePath)): config_dict = read_json_file(config) warnings.warn( - "Passsing a config file path is deprecated. Please pass the config " + "Passing a config file path is deprecated. Please pass the config " "as a dictionary instead.", SingerSDKDeprecationWarning, stacklevel=2, @@ -179,7 +178,7 @@ def __init__( # list will override those of earlier ones. config_dict.update(read_json_file(config_path)) warnings.warn( - "Passsing a list of config file paths is deprecated. Please pass the " + "Passing a list of config file paths is deprecated. Please pass the " "config as a dictionary instead.", SingerSDKDeprecationWarning, stacklevel=2, @@ -188,7 +187,7 @@ def __init__( config_dict = config else: msg = f"Error parsing config of type '{type(config).__name__}'." # type: ignore[unreachable] - raise ValueError(msg) + raise TypeError(msg) if parse_env_config: self.logger.info("Parsing env var for settings config...") config_dict.update(self._env_var_config) diff --git a/singer_sdk/tap_base.py b/singer_sdk/tap_base.py index c26fb823f..cccbe4fd5 100644 --- a/singer_sdk/tap_base.py +++ b/singer_sdk/tap_base.py @@ -106,7 +106,7 @@ def __init__( elif catalog is not None: self._input_catalog = Catalog.from_dict(read_json_file(catalog)) warnings.warn( - "Passsing a catalog file path is deprecated. Please pass the catalog " + "Passing a catalog file path is deprecated. Please pass the catalog " "as a dictionary or Catalog object instead.", SingerSDKDeprecationWarning, stacklevel=2, @@ -124,7 +124,7 @@ def __init__( elif state: state_dict = read_json_file(state) warnings.warn( - "Passsing a state file path is deprecated. Please pass the state " + "Passing a state file path is deprecated. Please pass the state " "as a dictionary instead.", SingerSDKDeprecationWarning, stacklevel=2, From 6aee7da999239725e093bf41788dda3cafc085c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez-Mondrag=C3=B3n?= Date: Mon, 2 Dec 2024 10:21:10 -0600 Subject: [PATCH 2/3] Test with invalid config type --- tests/core/test_plugin_base.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/core/test_plugin_base.py b/tests/core/test_plugin_base.py index 04b6a1665..2bc30ae2e 100644 --- a/tests/core/test_plugin_base.py +++ b/tests/core/test_plugin_base.py @@ -16,6 +16,12 @@ class PluginTest(PluginBase): ).to_dict() +def test_invalid_config_type(): + """Test that invalid config types raise an error.""" + with pytest.raises(TypeError, match="Error parsing config of type 'tuple'"): + PluginTest(config=(("prop1", "hello"), ("prop2", 123))) + + def test_get_env_var_config(monkeypatch: pytest.MonkeyPatch): """Test settings parsing from environment variables.""" monkeypatch.delenv("PLUGIN_TEST_PROP1", raising=False) From 2d6b36f587445c40ff8db129e56998364afd8d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Ram=C3=ADrez-Mondrag=C3=B3n?= Date: Mon, 2 Dec 2024 10:31:14 -0600 Subject: [PATCH 3/3] Test with config path --- tests/core/test_plugin_base.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/core/test_plugin_base.py b/tests/core/test_plugin_base.py index 2bc30ae2e..7f47435cd 100644 --- a/tests/core/test_plugin_base.py +++ b/tests/core/test_plugin_base.py @@ -1,10 +1,15 @@ from __future__ import annotations +import typing as t + import pytest from singer_sdk.plugin_base import SDK_PACKAGE_NAME, MapperNotInitialized, PluginBase from singer_sdk.typing import IntegerType, PropertiesList, Property, StringType +if t.TYPE_CHECKING: + from pathlib import Path + class PluginTest(PluginBase): """Example Plugin for tests.""" @@ -16,6 +21,18 @@ class PluginTest(PluginBase): ).to_dict() +def test_config_path(tmp_path: Path): + """Test that the config path is correctly set.""" + config_json = '{"prop1": "hello", "prop2": 123}' + config_path = tmp_path / "config.json" + config_path.write_text(config_json) + + with pytest.deprecated_call(): + plugin = PluginTest(config=config_path) + + assert plugin.config == {"prop1": "hello", "prop2": 123} + + def test_invalid_config_type(): """Test that invalid config types raise an error.""" with pytest.raises(TypeError, match="Error parsing config of type 'tuple'"):