Skip to content

Commit

Permalink
fix: Ensure the required global folder tap settings are merged into t…
Browse files Browse the repository at this point in the history
…he 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
  • Loading branch information
edgarrmondragon authored Dec 2, 2024
1 parent 0ac558a commit edcbb1a
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 13 deletions.
15 changes: 11 additions & 4 deletions singer_sdk/contrib/filesystem/tap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
13 changes: 6 additions & 7 deletions singer_sdk/plugin_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/tap_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions tests/core/test_plugin_base.py
Original file line number Diff line number Diff line change
@@ -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."""
Expand All @@ -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)
Expand Down

0 comments on commit edcbb1a

Please sign in to comment.