Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure the required global folder tap settings are merged into the concrete implementation settings #2790

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading