Skip to content

Commit

Permalink
Get rid of empty warnings list
Browse files Browse the repository at this point in the history
  • Loading branch information
edgarrmondragon committed Aug 9, 2022
1 parent 07464d7 commit 9e52cd8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 54 deletions.
3 changes: 0 additions & 3 deletions singer_sdk/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,15 @@ def __init__(
message: str,
*,
errors: list[str] | None = None,
warnings: list[str] | None = None,
) -> None:
"""Initialize a ConfigValidationError.
Args:
message: A message describing the error.
errors: A list of errors which caused the validation error.
warnings: A list of warnings which caused the validation error.
"""
super().__init__(message)
self.errors = errors or []
self.warnings = warnings or []


class FatalAPIError(Exception):
Expand Down
56 changes: 15 additions & 41 deletions singer_sdk/plugin_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,10 @@
from collections import OrderedDict
from pathlib import PurePath
from types import MappingProxyType
from typing import (
Any,
Callable,
Dict,
List,
Mapping,
Optional,
Tuple,
Type,
Union,
cast,
)
from typing import Any, Callable, Dict, List, Mapping, Optional, Type, Union, cast

import click
from jsonschema import Draft4Validator, SchemaError, ValidationError
from jsonschema import Draft4Validator

from singer_sdk.configuration._dict_config import parse_environment_config
from singer_sdk.exceptions import ConfigValidationError
Expand Down Expand Up @@ -215,35 +204,29 @@ 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
errors: List[str] = []

if config_jsonschema:
self.append_builtin_config(config_jsonschema)
try:
self.logger.debug(
f"Validating config using jsonschema: {config_jsonschema}"
)
validator = JSONSchemaValidator(config_jsonschema)
validator.validate(self._config)
except (ValidationError, SchemaError) as ex:
errors.append(str(ex.message))
self.logger.debug(
f"Validating config using jsonschema: {config_jsonschema}"
)
validator = JSONSchemaValidator(config_jsonschema)
errors = [error.message for error in validator.iter_errors(self._config)]

if errors:
summary = (
f"Config validation failed: {f'; '.join(errors)}\n"
Expand All @@ -252,18 +235,9 @@ def _validate_config(
if raise_errors:
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}"
if warnings_as_errors and raise_errors and warnings:
raise ConfigValidationError(
f"One or more warnings ocurred during validation: {warnings}",
warnings=warnings,
)
log_fn(summary)
return warnings, errors
self.logger.warning(summary)

return errors

@classmethod
def print_version(
Expand Down
4 changes: 1 addition & 3 deletions singer_sdk/tap_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,7 @@ def cli(
except ConfigValidationError as exc:
for error in exc.errors:
click.secho(error, fg="red")
for warning in exc.warnings:
click.secho(warning, fg="warning")
raise click.Abort("Configuration is not valid.")
raise click.Abort()

if discover:
tap.run_discovery()
Expand Down
18 changes: 12 additions & 6 deletions singer_sdk/target_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from joblib import Parallel, delayed, parallel_backend

from singer_sdk.cli import common_options
from singer_sdk.exceptions import RecordsWitoutSchemaException
from singer_sdk.exceptions import ConfigValidationError, RecordsWitoutSchemaException
from singer_sdk.helpers._classproperty import classproperty
from singer_sdk.helpers._compat import final
from singer_sdk.helpers.capabilities import CapabilitiesEnum, PluginCapabilities
Expand Down Expand Up @@ -505,6 +505,7 @@ def cli(
Raises:
FileNotFoundError: If the config file does not exist.
Abort: If the configuration is not valid.
"""
if version:
cls.print_version()
Expand Down Expand Up @@ -537,11 +538,16 @@ def cli(

config_files.append(Path(config_path))

target = cls( # type: ignore # Ignore 'type not callable'
config=config_files or None,
parse_env_config=parse_env_config,
validate_config=validate_config,
)
try:
target = cls( # type: ignore # Ignore 'type not callable'
config=config_files or None,
parse_env_config=parse_env_config,
validate_config=validate_config,
)
except ConfigValidationError as exc:
for error in exc.errors:
click.secho(error, fg="red")
raise click.Abort()

target.listen(file_input)

Expand Down
19 changes: 18 additions & 1 deletion tests/core/test_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

from __future__ import annotations

import json
import logging
from typing import Any, Iterable, cast

import pendulum
import pytest
import requests
from click.testing import CliRunner

from singer_sdk.exceptions import ConfigValidationError
from singer_sdk.helpers._classproperty import classproperty
Expand Down Expand Up @@ -406,7 +408,7 @@ def calculate_test_cost(
[
(
{},
["'username' is a required property"],
["'username' is a required property", "'password' is a required property"],
),
(
{"username": "utest"},
Expand All @@ -428,3 +430,18 @@ def test_config_errors(config_dict: dict, errors: list[str]):
SimpleTestTap(config_dict, validate_config=True)

assert exc.value.errors == errors


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

config_path = tmp_path / "config.json"
config_path.write_text(json.dumps({}))
result = runner.invoke(SimpleTestTap.cli, ["--config", str(config_path)])
assert result.exit_code == 1
assert "'username' is a required property" in result.output
assert "'password' is a required property" in result.output

0 comments on commit 9e52cd8

Please sign in to comment.