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

feat: Better error messages when config validation fails #768

Merged
merged 39 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
108a87c
feat: Validate unsupported config options
edgarrmondragon Jun 30, 2022
cd13367
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jun 30, 2022
9b59f61
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 1, 2022
d1c3fa8
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 6, 2022
07464d7
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 9, 2022
c05087e
Get rid of empty warnings list
edgarrmondragon Aug 9, 2022
03ca8e0
Test --discover should not raise error
edgarrmondragon Aug 9, 2022
ad77afa
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 23, 2022
90cc541
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 23, 2022
39af208
Tests cleanup
edgarrmondragon Aug 23, 2022
7e1d10a
Add target class init test
edgarrmondragon Aug 23, 2022
a05c61f
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 23, 2022
487ad8f
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 24, 2022
8b8e75c
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 24, 2022
f79e5bb
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 24, 2022
63b8e13
Add JSONTypeHelper.to_json wrapper
edgarrmondragon Aug 25, 2022
965dbb2
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Aug 31, 2022
7caa5c8
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Sep 14, 2022
f3ccdc3
Test mapper class configuration
edgarrmondragon Sep 14, 2022
a7af1e8
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Sep 14, 2022
b2f6366
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Sep 15, 2022
5c47e8d
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Feb 4, 2023
ee039fc
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Feb 16, 2023
03d1cae
Split the CLI tests
edgarrmondragon Feb 16, 2023
77cfca9
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Feb 22, 2023
79b7878
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Feb 24, 2023
968c6d2
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Mar 25, 2023
ef04031
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Apr 25, 2023
5b8d67c
Get rid of color
edgarrmondragon Apr 25, 2023
3fb6dfb
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 18, 2023
624a658
Address lint issues
edgarrmondragon Jul 18, 2023
2852dbe
Do not validate config in selection test
edgarrmondragon Jul 18, 2023
94a7cfa
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 19, 2023
5889975
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 19, 2023
8fb99fd
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Jul 19, 2023
e8443f8
Revert unnecessary change
edgarrmondragon Jul 20, 2023
05dc8d6
Merge branch 'main' into 753-validate-taptarget-config-options
edgarrmondragon Nov 17, 2023
96b8193
Address lint issues
edgarrmondragon Nov 17, 2023
93e29c0
Use logger instead of click.secho
edgarrmondragon Nov 17, 2023
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
1 change: 1 addition & 0 deletions samples/sample_tap_sqlite/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class SQLiteTap(SQLTap):
DB_PATH_CONFIG,
th.StringType,
description="The path to your SQLite database file(s).",
required=True,
examples=["./path/to/my.db", "/absolute/path/to/my.db"],
),
).to_dict()
Expand Down
1 change: 1 addition & 0 deletions samples/sample_target_sqlite/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class SQLiteTarget(SQLTarget):
DB_PATH_CONFIG,
th.StringType,
description="The path to your SQLite database file(s).",
required=True,
),
).to_dict()

Expand Down
15 changes: 15 additions & 0 deletions singer_sdk/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@
class ConfigValidationError(Exception):
"""Raised when a user's config settings fail validation."""

def __init__(
self,
message: str,
*,
errors: list[str] | None = None,
edgarrmondragon marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
"""Initialize a ConfigValidationError.

Args:
message: A message describing the error.
errors: A list of errors which caused the validation error.
"""
super().__init__(message)
self.errors = errors or []


class FatalAPIError(Exception):
"""Exception raised when a failed request should not be considered retriable."""
Expand Down
72 changes: 47 additions & 25 deletions singer_sdk/plugin_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,43 @@ def __init__(self) -> None:
super().__init__("Mapper not initialized. Please call setup_mapper() first.")


class SingerCommand(click.Command):
"""Custom click command class for Singer packages."""

def __init__(
self,
*args: t.Any,
logger: logging.Logger,
**kwargs: t.Any,
) -> None:
"""Initialize the command.

Args:
*args: Positional `click.Command` arguments.
logger: A logger instance.
**kwargs: Keyword `click.Command` arguments.
"""
super().__init__(*args, **kwargs)
self.logger = logger

def invoke(self, ctx: click.Context) -> t.Any: # noqa: ANN401
"""Invoke the command, capturing warnings and logging them.

Args:
ctx: The `click` context.

Returns:
The result of the command invocation.
"""
logging.captureWarnings(capture=True)
try:
return super().invoke(ctx)
except ConfigValidationError as exc:
for error in exc.errors:
self.logger.error("Config validation error: %s", error)
sys.exit(1)


class PluginBase(metaclass=abc.ABCMeta):
"""Abstract base class for taps."""

Expand Down Expand Up @@ -150,12 +187,12 @@ def __init__(
if self._is_secret_config(k):
config_dict[k] = SecretString(v)
self._config = config_dict
self._validate_config(raise_errors=validate_config)
self._mapper: PluginMapper | None = None

metrics._setup_logging(self.config)
self.metrics_logger = metrics.get_metrics_logger()

self._validate_config(raise_errors=validate_config)
self._mapper: PluginMapper | None = None

# Initialization timestamp
self.__initialized_at = int(time.time() * 1000)

Expand Down Expand Up @@ -351,27 +388,19 @@ def _is_secret_config(config_key: str) -> bool:
"""
return is_common_secret_key(config_key)

def _validate_config(
self,
*,
raise_errors: bool = True,
warnings_as_errors: bool = False,
) -> tuple[list[str], list[str]]:
def _validate_config(self, *, raise_errors: bool = True) -> list[str]:
"""Validate configuration input against the plugin configuration JSON schema.

Args:
raise_errors: Flag to throw an exception if any validation errors are found.
warnings_as_errors: Flag to throw an exception if any warnings were emitted.

Returns:
A tuple of configuration validation warnings and errors.
A list of validation errors.

Raises:
ConfigValidationError: If raise_errors is True and validation fails.
"""
warnings: list[str] = []
errors: list[str] = []
log_fn = self.logger.info
config_jsonschema = self.config_jsonschema

if config_jsonschema:
Expand All @@ -389,19 +418,11 @@ def _validate_config(
f"JSONSchema was: {config_jsonschema}"
)
if raise_errors:
raise ConfigValidationError(summary)
raise ConfigValidationError(summary, errors=errors)

log_fn = self.logger.warning
else:
summary = f"Config validation passed with {len(warnings)} warnings."
for warning in warnings:
summary += f"\n{warning}"
self.logger.warning(summary)

if warnings_as_errors and raise_errors and warnings:
msg = f"One or more warnings ocurred during validation: {warnings}"
raise ConfigValidationError(msg)
log_fn(summary)
return warnings, errors
return errors

@classmethod
def print_version(
Expand Down Expand Up @@ -555,7 +576,7 @@ def get_singer_command(cls: type[PluginBase]) -> click.Command:
Returns:
A callable CLI object.
"""
return click.Command(
return SingerCommand(
name=cls.name,
callback=cls.invoke,
context_settings={"help_option_names": ["--help"]},
Expand Down Expand Up @@ -596,6 +617,7 @@ def get_singer_command(cls: type[PluginBase]) -> click.Command:
is_eager=True,
),
],
logger=cls.logger,
)

@plugin_cli
Expand Down
101 changes: 101 additions & 0 deletions tests/core/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
"""Tap, target and stream test fixtures."""

from __future__ import annotations

import typing as t

import pendulum
import pytest

from singer_sdk import Stream, Tap
from singer_sdk.typing import (
DateTimeType,
IntegerType,
PropertiesList,
Property,
StringType,
)


class SimpleTestStream(Stream):
"""Test stream class."""

name = "test"
schema = PropertiesList(
Property("id", IntegerType, required=True),
Property("value", StringType, required=True),
Property("updatedAt", DateTimeType, required=True),
).to_dict()
replication_key = "updatedAt"

def __init__(self, tap: Tap):
"""Create a new stream."""
super().__init__(tap, schema=self.schema, name=self.name)

def get_records(
self,
context: dict | None, # noqa: ARG002
) -> t.Iterable[dict[str, t.Any]]:
"""Generate records."""
yield {"id": 1, "value": "Egypt"}
yield {"id": 2, "value": "Germany"}
yield {"id": 3, "value": "India"}


class UnixTimestampIncrementalStream(SimpleTestStream):
name = "unix_ts"
schema = PropertiesList(
Property("id", IntegerType, required=True),
Property("value", StringType, required=True),
Property("updatedAt", IntegerType, required=True),
).to_dict()
replication_key = "updatedAt"


class UnixTimestampIncrementalStream2(UnixTimestampIncrementalStream):
name = "unix_ts_override"

def compare_start_date(self, value: str, start_date_value: str) -> str:
"""Compare a value to a start date value."""

start_timestamp = pendulum.parse(start_date_value).format("X")
return max(value, start_timestamp, key=float)


class SimpleTestTap(Tap):
"""Test tap class."""

name = "test-tap"
config_jsonschema = PropertiesList(
Property("username", StringType, required=True),
Property("password", StringType, required=True),
Property("start_date", DateTimeType),
additional_properties=False,
).to_dict()

def discover_streams(self) -> list[Stream]:
"""List all streams."""
return [
SimpleTestStream(self),
UnixTimestampIncrementalStream(self),
UnixTimestampIncrementalStream2(self),
]


@pytest.fixture
def tap_class():
"""Return the tap class."""
return SimpleTestTap


@pytest.fixture
def tap() -> SimpleTestTap:
"""Tap instance."""
return SimpleTestTap(
config={
"username": "utest",
"password": "ptest",
"start_date": "2021-01-01",
},
parse_env_config=False,
)
54 changes: 54 additions & 0 deletions tests/core/test_mapper_class.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
from __future__ import annotations

import json
from contextlib import nullcontext

import pytest
from click.testing import CliRunner

from samples.sample_mapper.mapper import StreamTransform
from singer_sdk.exceptions import ConfigValidationError


@pytest.mark.parametrize(
"config_dict,expectation,errors",
[
pytest.param(
{},
pytest.raises(ConfigValidationError, match="Config validation failed"),
["'stream_maps' is a required property"],
id="missing_stream_maps",
),
pytest.param(
{"stream_maps": {}},
nullcontext(),
[],
id="valid_config",
),
],
)
def test_config_errors(config_dict: dict, expectation, errors: list[str]):
with expectation as exc:
StreamTransform(config=config_dict, validate_config=True)

if isinstance(exc, pytest.ExceptionInfo):
assert exc.value.errors == errors


def test_cli_help():
"""Test the CLI help message."""
runner = CliRunner(mix_stderr=False)
result = runner.invoke(StreamTransform.cli, ["--help"])
assert result.exit_code == 0
assert "Show this message and exit." in result.output


def test_cli_config_validation(tmp_path):
"""Test the CLI config validation."""
runner = CliRunner(mix_stderr=False)
config_path = tmp_path / "config.json"
config_path.write_text(json.dumps({}))
result = runner.invoke(StreamTransform.cli, ["--config", str(config_path)])
assert result.exit_code == 1
assert not result.stdout
assert "'stream_maps' is a required property" in result.stderr
Loading