From edcbb1ae35745ca73cd01668b82689360b44cfab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Edgar=20Ram=C3=ADrez=20Mondrag=C3=B3n?=
<16805946+edgarrmondragon@users.noreply.github.com>
Date: Mon, 2 Dec 2024 10:39:32 -0600
Subject: [PATCH] fix: Ensure the required global folder tap settings are
merged into the concrete implementation settings (#2790)
* fix: Ensure the required global folder tap settings are merged into the concrete implementation settings
* Test with invalid config type
* Test with config path
---
singer_sdk/contrib/filesystem/tap.py | 15 +++++++++++----
singer_sdk/plugin_base.py | 13 ++++++-------
singer_sdk/tap_base.py | 4 ++--
tests/core/test_plugin_base.py | 23 +++++++++++++++++++++++
4 files changed, 42 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,
diff --git a/tests/core/test_plugin_base.py b/tests/core/test_plugin_base.py
index 04b6a1665..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,24 @@ 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'"):
+ 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)